Akka-http: Relax user-agent parsing to fit reality

[gaydenko]

Hi! I get:

Illegal request header: Illegal 'user-agent' header: Invalid input '/', expected OWS, 'EOI', tchar, product-or-comment or comment (line 1, column 184): Mozilla/5.0 (Linux; U; Android 5.0.2; en-us; Redmi Note 3 Build/LRX22G) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/53.0.2785.146 Mobile Safari/537.36 XiaoMi/MiuiBrowser/8.4.6

So, the parser is too strict to fit reality.

[viktorklang]

# Enables/disables the logging of warning messages in case an incoming
# message (request or response) contains an HTTP header which cannot be
# parsed into its high-level model class due to incompatible syntax.
# Note that, independently of this settings, akka-http will accept messages
# with such headers as long as the message as a whole would still be legal
# under the HTTP specification even without this header.
# If a header cannot be parsed into a high-level model instance it will be
# provided as a `RawHeader`.
# If logging is enabled it is performed with the configured
# `error-logging-verbosity`.
illegal-header-warnings = on

http://doc.akka.io/docs/akka-http/current/scala/http/configuration.html

[gaydenko]

I would prefer to keep other headers-related warnings not-suppressed.

[viktorklang]

? The warning is logged due to a spec violation.

[gaydenko]

Yes, I'm saying just about that. When millions devices don't follow a specification, I guess it is more handy for software to follow reality rather the (just still not updated) specification :)

[ktoso]

I do agree that the world of http is a mess. The user agent warnings are
very exposed to a mess that is the world of crappy clients, thus we could
change the handling as well as optional settings for specifically that
header.

On Dec 26, 2016 18:10, "Andrew Gaydenko" [e-mail] wrote:

Yes, I'm saying just about that. When millions devices don't follow a
specification, I guess it is more handy for software to follow reality
rather the (just still not updated) specification :)

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/akka/akka-http/issues/687#issuecomment-269227748, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAHYk9lRGFcHKumLjpOjgoqDmEzV6xGKks5rL_UBgaJpZM4LVwai
.

[jrudolph]

I agree we might relax it if it really happens for lots of people. Is the XiaoMi/MiuiBrowser really as prominent as you say? Could we also file an issue with them to fix their user-agent header as well?

[gaydenko]

@jrudolph Sorry, I guess it may be difficult to get some statistics related to Xiaomi spread. It is one of the most popular China manufacturer, and the browser is the official stock one.

I get the warning as log file viewer rather than as device owner :)

[jrudolph]

I see, even if they would fix it now the problem right might still persist for years to come.

[jrudolph]

Btw. I still don't necessarily see it as worthy of a special case. I think, overall, the number of non-conformant clients isn't particularly high. But in your niche you might get all kinds of prominent problems. So, in the best case, we could allow some kind of configurable warning filter that would allow you to filter out warnings based on key/value pairs of unparseable headers. WDYT?

[gaydenko]

Agree, more specialized filtering would be handy decision for this issue as well as for the others similar possible ones.

[akara]

Let me add to this bug, just when we wanted to turn an application live we found such messages:

2017-05-26 13:07:01,247 WARN [squbs-akka.actor.default-dispatcher-15] ActorSystemImpl akka.actor.ActorSystemImpl(squbs) Illegal request header: Illegal 'user-agent' header: Invalid input '[', expected 'EOI', product-or-comment, WSP, comment or CRLF (line 1, column 104): Mozilla/5.0 (iPad; CPU OS 10_3_1 like Mac OS X) AppleWebKit/603.1.30 (KHTML, like Gecko) Mobile/14E304 [FBAN/FBIOS;FBAV/93.0.0.49.65;FBBV/58440824;FBDV/iPad4,1;FBMD/iPad;FBSN/iOS;FBSV/10.3.1;FBSS/2;FBCR/;FBID/tablet;FBLC/fr_FR;FBOP/5;FBRV/0]

Now, clearly the [...] is rejected. Is this a prevalent issue? We found this StackOverflow article especially interesting: https://stackoverflow.com/questions/11414006/is-this-a-facebook-for-ios-webview-user-agent

So every call from the Facebook client on iOS is generating this. While I don't have the exact stats, I have the gut feeling quite a lot of people are using the Facebook app on iOS, don't you think?

Yes, we can turn the warnings off. But does this mean we have to capture the User-Agent from RawHeader as well as the formally typed User-Agent header? And yes, the application is interested in all the contents of User-Agent.

[teenasharma]

Hi,

Is there any resolution to this problem because we are getting literally thousands of such logs for various browsers, mostly from iOS and android.

[WARN] from akka.actor.ActorSystemImpl in application-akka.actor.default-dispatcher-4655 - Illegal header: Illegal 'user-agent' header: Invalid input '[', expected 'EOI', product-or-comment, WSP, comment or CRLF (line 1, column 113): Mozilla/5.0 (iPhone; CPU iPhone OS 11_0_2 like Mac OS X) AppleWebKit/604.1.38 (KHTML, like Gecko) Mobile/15A421 [FBAN/FBIOS;FBAV/141.0.0.51.91;FBBV/71695290;FBDV/iPhone9,3;FBMD/iPhone;FBSN/iOS;FBSV/11.0.2;FBSS/2;FBCR/AT&T;FBID/phone;FBLC/en_US;FBOP/5;FBRV/72701227]

[raboof]

@teenasharma as commented above you can set illegal-header-warnings to false. This does mean warnings about other headers are also suppressed, which is what this ticket (now) is about.

[gmethvin]

@raboof Does this mean akka-http just drops the header then? Obviously just ignoring the warning is not a nice solution. We are seeing Play users report this problem as well.

[jatcwang]

I'd like to help fix this. A more general solution to this problem is to allow the user to set a list of headers they don't want invalid warnings to be logged against. Config name will be something like ignore-illegal-header-for.

In HttpHeaderParser#defaultIllegalHeaderHandler we'll simply add a check against the set of headers to ignore and not log if it matches.

What do you think?

[raboof]

@gmethvin the details are in HttpHeaderParser, I think if the header could not be parsed it will not be dropped but included as a RawHeader instead of a header of the expected type.

[raboof]

@jatcwang that sounds like a useful contribution! One thing to be careful about might be to make sure this doesn't have any impact on the performance in the common/critical path, but that sounds doable.

[ktoso]

Ability to disable warnings for certain headers has now been merged