You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Jordan Ly <jo...@gmail.com> on 2018/02/16 20:59:14 UTC
Review Request 65690: Make charset parsing in HTTP headers case
insensitve
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65690/
-----------------------------------------------------------
Review request for Aurora, David McLaughlin, Renan DelValle, Santhosh Kumar Shanmugham, and Stephan Erb.
Repository: aurora
Description
-------
Users have reported that the UI does not load in Firefox. Investigating the issue shows that Chrome and Safari will format charset into an uppercase `UTF-8` which is accepted by the servlet. However, Mozilla will leave the charset as lowercase `utf-8` which causes a 415 response.
Charsets should be case-insensitive but the default Java `MediaType` class does not take this into account when parsing/comparing. I propose switching to Guava's `MediaType` class which does smarter comparisons.
Diffs
-----
src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2820cda42293649e02eff6122621d485cb803ac2
src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java d9df96896d83786596880105968dc7fcdc0168e5
src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java bfd117b4988c35c6b3f95508b2924bbd19b0c692
Diff: https://reviews.apache.org/r/65690/diff/1/
Testing
-------
`./gradlew test`
Manual testing in Safari, Chrome, and Firefox.
Thanks,
Jordan Ly
Re: Review Request 65690: Make charset parsing in HTTP headers case
insensitve
Posted by Jordan Ly <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65690/#review197688
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
Line 124 (original), 126 (patched)
<https://reviews.apache.org/r/65690/#comment277921>
Note to reviewers: this differes from APPLICATION_JSON_TYPE because it specifies a charset. I believe this should not affect us after reading https://github.com/google/guava/issues/915, but if anyone has a way to confirm please let me know :)
- Jordan Ly
On Feb. 16, 2018, 8:59 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65690/
> -----------------------------------------------------------
>
> (Updated Feb. 16, 2018, 8:59 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Renan DelValle, Santhosh Kumar Shanmugham, and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Users have reported that the UI does not load in Firefox. Investigating the issue shows that Chrome and Safari will format charset into an uppercase `UTF-8` which is accepted by the servlet. However, Mozilla will leave the charset as lowercase `utf-8` which causes a 415 response.
>
> Charsets should be case-insensitive but the default Java `MediaType` class does not take this into account when parsing/comparing. I propose switching to Guava's `MediaType` class which does smarter comparisons.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2820cda42293649e02eff6122621d485cb803ac2
> src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java d9df96896d83786596880105968dc7fcdc0168e5
> src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java bfd117b4988c35c6b3f95508b2924bbd19b0c692
>
>
> Diff: https://reviews.apache.org/r/65690/diff/1/
>
>
> Testing
> -------
>
> `./gradlew test`
> Manual testing in Safari, Chrome, and Firefox.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65690: Make charset parsing in HTTP headers case
insensitve
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65690/#review197692
-----------------------------------------------------------
Ship it!
Master (59f66ff) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Feb. 16, 2018, 8:59 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65690/
> -----------------------------------------------------------
>
> (Updated Feb. 16, 2018, 8:59 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Renan DelValle, Santhosh Kumar Shanmugham, and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Users have reported that the UI does not load in Firefox. Investigating the issue shows that Chrome and Safari will format charset into an uppercase `UTF-8` which is accepted by the servlet. However, Mozilla will leave the charset as lowercase `utf-8` which causes a 415 response.
>
> Charsets should be case-insensitive but the default Java `MediaType` class does not take this into account when parsing/comparing. I propose switching to Guava's `MediaType` class which does smarter comparisons.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2820cda42293649e02eff6122621d485cb803ac2
> src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java d9df96896d83786596880105968dc7fcdc0168e5
> src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java bfd117b4988c35c6b3f95508b2924bbd19b0c692
>
>
> Diff: https://reviews.apache.org/r/65690/diff/1/
>
>
> Testing
> -------
>
> `./gradlew test`
> Manual testing in Safari, Chrome, and Firefox.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65690: Make charset parsing in HTTP headers case
insensitve
Posted by Jordan Ly <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65690/#review197738
-----------------------------------------------------------
src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
Line 124 (original), 126 (patched)
<https://reviews.apache.org/r/65690/#comment277993>
Update: opted to remove charset to retain current behavior.
- Jordan Ly
On Feb. 19, 2018, 6:34 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65690/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2018, 6:34 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Renan DelValle, Santhosh Kumar Shanmugham, and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Users have reported that the UI does not load in Firefox. Investigating the issue shows that Chrome and Safari will format charset into an uppercase `UTF-8` which is accepted by the servlet. However, Mozilla will leave the charset as lowercase `utf-8` which causes a 415 response.
>
> Charsets should be case-insensitive but the default Java `MediaType` class does not take this into account when parsing/comparing. I propose switching to Guava's `MediaType` class which does smarter comparisons.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2820cda42293649e02eff6122621d485cb803ac2
> src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java d9df96896d83786596880105968dc7fcdc0168e5
> src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java bfd117b4988c35c6b3f95508b2924bbd19b0c692
>
>
> Diff: https://reviews.apache.org/r/65690/diff/2/
>
>
> Testing
> -------
>
> `./gradlew test`
> Manual testing in Safari, Chrome, and Firefox.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65690: Make charset parsing in HTTP headers case
insensitve
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65690/#review197739
-----------------------------------------------------------
Ship it!
Master (59f66ff) is green with this patch.
./build-support/jenkins/build.sh
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Feb. 19, 2018, 6:34 a.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65690/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2018, 6:34 a.m.)
>
>
> Review request for Aurora, David McLaughlin, Renan DelValle, Santhosh Kumar Shanmugham, and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Users have reported that the UI does not load in Firefox. Investigating the issue shows that Chrome and Safari will format charset into an uppercase `UTF-8` which is accepted by the servlet. However, Mozilla will leave the charset as lowercase `utf-8` which causes a 415 response.
>
> Charsets should be case-insensitive but the default Java `MediaType` class does not take this into account when parsing/comparing. I propose switching to Guava's `MediaType` class which does smarter comparisons.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2820cda42293649e02eff6122621d485cb803ac2
> src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java d9df96896d83786596880105968dc7fcdc0168e5
> src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java bfd117b4988c35c6b3f95508b2924bbd19b0c692
>
>
> Diff: https://reviews.apache.org/r/65690/diff/3/
>
>
> Testing
> -------
>
> `./gradlew test`
> Manual testing in Safari, Chrome, and Firefox.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65690: Make charset parsing in HTTP headers case
insensitve
Posted by Jordan Ly <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65690/
-----------------------------------------------------------
(Updated Feb. 19, 2018, 6:34 a.m.)
Review request for Aurora, David McLaughlin, Renan DelValle, Santhosh Kumar Shanmugham, and Stephan Erb.
Changes
-------
Use 'application/json' without charset specified
Repository: aurora
Description
-------
Users have reported that the UI does not load in Firefox. Investigating the issue shows that Chrome and Safari will format charset into an uppercase `UTF-8` which is accepted by the servlet. However, Mozilla will leave the charset as lowercase `utf-8` which causes a 415 response.
Charsets should be case-insensitive but the default Java `MediaType` class does not take this into account when parsing/comparing. I propose switching to Guava's `MediaType` class which does smarter comparisons.
Diffs (updated)
-----
src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2820cda42293649e02eff6122621d485cb803ac2
src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java d9df96896d83786596880105968dc7fcdc0168e5
src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java bfd117b4988c35c6b3f95508b2924bbd19b0c692
Diff: https://reviews.apache.org/r/65690/diff/2/
Changes: https://reviews.apache.org/r/65690/diff/1-2/
Testing
-------
`./gradlew test`
Manual testing in Safari, Chrome, and Firefox.
Thanks,
Jordan Ly
Re: Review Request 65690: Make charset parsing in HTTP headers case
insensitve
Posted by Santhosh Kumar Shanmugham <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65690/#review197695
-----------------------------------------------------------
Ship it!
Ship It!
- Santhosh Kumar Shanmugham
On Feb. 16, 2018, 12:59 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65690/
> -----------------------------------------------------------
>
> (Updated Feb. 16, 2018, 12:59 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Renan DelValle, Santhosh Kumar Shanmugham, and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Users have reported that the UI does not load in Firefox. Investigating the issue shows that Chrome and Safari will format charset into an uppercase `UTF-8` which is accepted by the servlet. However, Mozilla will leave the charset as lowercase `utf-8` which causes a 415 response.
>
> Charsets should be case-insensitive but the default Java `MediaType` class does not take this into account when parsing/comparing. I propose switching to Guava's `MediaType` class which does smarter comparisons.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2820cda42293649e02eff6122621d485cb803ac2
> src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java d9df96896d83786596880105968dc7fcdc0168e5
> src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java bfd117b4988c35c6b3f95508b2924bbd19b0c692
>
>
> Diff: https://reviews.apache.org/r/65690/diff/1/
>
>
> Testing
> -------
>
> `./gradlew test`
> Manual testing in Safari, Chrome, and Firefox.
>
>
> Thanks,
>
> Jordan Ly
>
>
Re: Review Request 65690: Make charset parsing in HTTP headers case
insensitve
Posted by Renan DelValle <re...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65690/#review197694
-----------------------------------------------------------
Ship it!
Ship It!
- Renan DelValle
On Feb. 16, 2018, 12:59 p.m., Jordan Ly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65690/
> -----------------------------------------------------------
>
> (Updated Feb. 16, 2018, 12:59 p.m.)
>
>
> Review request for Aurora, David McLaughlin, Renan DelValle, Santhosh Kumar Shanmugham, and Stephan Erb.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Users have reported that the UI does not load in Firefox. Investigating the issue shows that Chrome and Safari will format charset into an uppercase `UTF-8` which is accepted by the servlet. However, Mozilla will leave the charset as lowercase `utf-8` which causes a 415 response.
>
> Charsets should be case-insensitive but the default Java `MediaType` class does not take this into account when parsing/comparing. I propose switching to Guava's `MediaType` class which does smarter comparisons.
>
>
> Diffs
> -----
>
> src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2820cda42293649e02eff6122621d485cb803ac2
> src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java d9df96896d83786596880105968dc7fcdc0168e5
> src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java bfd117b4988c35c6b3f95508b2924bbd19b0c692
>
>
> Diff: https://reviews.apache.org/r/65690/diff/1/
>
>
> Testing
> -------
>
> `./gradlew test`
> Manual testing in Safari, Chrome, and Firefox.
>
>
> Thanks,
>
> Jordan Ly
>
>