-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37097/#review94958
-----------------------------------------------------------

Ship it!


Couple of comments below, but I'll make the adjustments and get this committed 
for you shortly! Thanks for splitting this out and updating the test!


3rdparty/libprocess/include/process/http.hpp (line 120)
<https://reviews.apache.org/r/37097/#comment149686>

    We're still wrapping comments at 70 for now, but that might change soon :)



3rdparty/libprocess/include/process/http.hpp (line 122)
<https://reviews.apache.org/r/37097/#comment149684>

    Which RFC? :)



3rdparty/libprocess/include/process/http.hpp (lines 124 - 125)
<https://reviews.apache.org/r/37097/#comment149683>

    This bit seems to just be re-iterating the summary above?



3rdparty/libprocess/src/http.cpp (lines 134 - 135)
<https://reviews.apache.org/r/37097/#comment149692>

    Reading through the RFC again, it's quite a bit more subtle than this:
    
    ```
    4. The "identity" content-coding is always acceptable, unless
       specifically refused because the Accept-Encoding field includes
       "identity;q=0", or because the field includes "*;q=0" and does
       not explicitly include the "identity" content-coding. If the
       Accept-Encoding field-value is empty, then only the "identity"
       encoding is acceptable.
    
    If no Accept-Encoding field is present in a request, the server MAY assume 
that the client will accept any content coding. In this case, if "identity" is 
one of the available content-codings, then the server SHOULD use the "identity" 
content-coding, unless it has additional information that a different 
content-coding is meaningful to the client.
    
          Note: If the request does not include an Accept-Encoding field,
          and if the "identity" content-coding is unavailable, then
          content-codings commonly understood by HTTP/1.0 clients (i.e.,
          "gzip" and "compress") are preferred; some older clients
          improperly display messages sent with other content-codings.  The
          server might also make this decision based on information about
          the particular user-agent or client.
          Note: Most HTTP/1.0 applications do not recognize or obey qvalues
          associated with content-codings. This means that qvalues will not
          work and are not permitted with x-gzip or x-compress.
    ```
    
    So it appears that we're doing the right thing here by returning false and 
using the identity encoding, but we don't correctly deal with explicitly 
rejected identity encoding for now.. I'll remove this and the TODO since it's a 
bit misleading



3rdparty/libprocess/src/http.cpp (lines 145 - 147)
<https://reviews.apache.org/r/37097/#comment149688>

    Shouldn't this be up where we're returning false..?



3rdparty/libprocess/src/http.cpp (line 159)
<https://reviews.apache.org/r/37097/#comment149701>

    Hm.. it's a bit implicit that tokens.empty is guaranteed to be false here 
because 'encoding' itself is non-empty due to it coming from tokenize.


- Ben Mahler


On Aug. 7, 2015, 6:52 p.m., Isabel Jimenez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37097/
> -----------------------------------------------------------
> 
> (Updated Aug. 7, 2015, 6:52 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently parsing only compares the begining of the header making 'gzipbug' 
> match with candidate 'gzip'
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp b8d9300 
>   3rdparty/libprocess/src/http.cpp 4dcbd74 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 0032137 
> 
> Diff: https://reviews.apache.org/r/37097/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Reply via email to