Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getForKeyCaseInsensitive not working on HttpMethods. #2303

Closed
liljaylj opened this issue Dec 4, 2018 · 1 comment
Closed

getForKeyCaseInsensitive not working on HttpMethods. #2303

liljaylj opened this issue Dec 4, 2018 · 1 comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Identifies issues that the core team will likely not have time to work on
Milestone

Comments

@liljaylj
Copy link
Contributor

liljaylj commented Dec 4, 2018

HttpMethods.getForKeyCaseInsensitive("POST") gives None, should be Some(HttpMethod(GET))

The reason is that when HttpMethods registers its members it uses upper case keys

private def register(method: HttpMethod): HttpMethod = register(method.value, method)

... but when invoking getForKeyCaseInsensitive under the hood it applies toRootLowerCase on key

My recommendations are either store keys in lower case or override getForKeyCaseInsensitive and replace toRootLowerCase with toUpperCase(Locale.ROOT)

override def getForKeyCaseInsensitive(key: String)(implicit conv: String <:< String) : Option[HttpMethod] =
    getForKey(key.toUpperCase(Locale.ROOT))
@raboof raboof added help wanted Identifies issues that the core team will likely not have time to work on 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted labels Dec 4, 2018
@raboof
Copy link
Member

raboof commented Dec 4, 2018

@liljaylj huh, good observation! I suppose replacing toRootLowerCase with toUpperCase(Locale.ROOT) would be the most unobtrusive change, unless we have reason to believe this method is performance-critical? Would you like to propose the change as a PR?

jlprat pushed a commit that referenced this issue Dec 5, 2018
* Overriden `getForKeyCaseInsensitive` in `HttpMethods` #2303

* Added tests for HttpMethods.getForKeyCaseInsensitive()

* Make validatePullRequest pass. Attempt 1
@jlprat jlprat added this to the 10.1.6 milestone Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted help wanted Identifies issues that the core team will likely not have time to work on
Projects
None yet
Development

No branches or pull requests

3 participants