Ignore trailing semicolons when parsing Accept-Language header

[schnapster]

Something I'm seeing in the wild is a trailing semicolon for Accept-Language headers. Partial example stack trace:

java.lang.IllegalArgumentException: range=en;
    at java.util.Locale$LanguageRange.<init>(Locale.java:3099)
    at sun.util.locale.LocaleMatcher.parse(LocaleMatcher.java:525)
    at java.util.Locale$LanguageRange.parse(Locale.java:3214)
    at org.springframework.http.HttpHeaders.getAcceptLanguage(HttpHeaders.java:505)
    at org.springframework.http.HttpHeaders.getAcceptLanguageAsLocales(HttpHeaders.java:526)

I don't think it would hurt to be more lenient here and drop the trailing semicolon before attempting to parse it.

[pivotal-cla]

@schnapster Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

[pivotal-cla]

@schnapster Thank you for signing the Contributor License Agreement!

[rstoyanchev]

@schnapster, the header is parsed via Locale.LanguageRange.parse, which supports a weighted list of ranges where each may have a q parameter. However, the parsing logic doesn't allow a lot of flexibility and looks specifically for ";q=". Note also it first splits by , to create a list of range value. So the problem may still occur if there is a trailing ; in one of the other ranges.

We don't want to replicate the parsing logic from Locale, but we may be able to do a split by , and then check for invalid ";" in each. However, this is better done defensively, i.e. allowing parsing to fail first and then reacting to that in fallback mode.

[jdicke]

';' should be a variable

[kmesiab]

While I think a null check is fine, why not also validate whether this string is non empty and at least [n] chars and avoid the noop?

[schnapster]

@rstoyanchev apologies for the late follow-up, I'm a bit out of the loop on this by now. Could you please provide a concrete header to demonstrate your example? Or ideally a (pseudo-cody) test case similar to the one I added? I'd be happy to take another stab at this if I understand better what the additional faults are that you want Spring to be able to handle. We can also handle that in another issue / PR, imho this one already provides incremental value by itself.

[poutsma]

@schnapster Thank you for submitting a PR. I have combined your code and the suggestions of @rstoyanchev into 099d016.