Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

2017-01-14 Thread Jie Yu

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




3rdparty/libprocess/src/decoder.hpp (lines 414 - 423)


This sounds like a hack. See my comments in the ticket. The http parser 
seemed to do the right thing in the sense that if there is no content length or 
tranfer encoding, it'll try to read until EOF.

In http::decodeResponses, we don't actually call `decode("", 0)` to signal 
the EOF, we should fix that first.

Then, I think we should workaround the problem at the callsite (i.e., 
docker fetcher).

What I am thinking right now is that we do some special handling when we 
know there is an HTTPS_PROXY. We can parse the 200 response, the body of which 
will be the actual response from the remote server. We need to call parse again 
on that.


- Jie Yu


On Jan. 13, 2017, 3:51 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55496/
> ---
> 
> (Updated Jan. 13, 2017, 3:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-6010
> https://issues.apache.org/jira/browse/MESOS-6010
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When behind a proxy, access to websites using SSL can be done by using
> HTTP CONNECT tunneling. This is, for example, used when we pull images
> from a Docker repository. The HTTP response of such a CONNECT request
> surfaced a problem with our HTTP parser, because it doesn't have neither
> any headers nor a message body. The parser expected a 'Content-Length'
> header to be set in every HTTP response. We fix this by assuming that
> a HTTP response doesn't have a message body when the 'Content-Length'
> header isn't set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 4c779d42548958e610142438a57529ccb4478053 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 
> 87563f4fcdfaa2a33c4482533ddff24e062e603a 
> 
> Diff: https://reviews.apache.org/r/55496/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

2017-01-13 Thread Jan Schlicht

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

(Updated Jan. 13, 2017, 4:51 p.m.)


Review request for mesos, Benjamin Bannier, Gilbert Song, and Jie Yu.


Changes
---

Addressed issues.


Bugs: MESOS-6010
https://issues.apache.org/jira/browse/MESOS-6010


Repository: mesos


Description
---

When behind a proxy, access to websites using SSL can be done by using
HTTP CONNECT tunneling. This is, for example, used when we pull images
from a Docker repository. The HTTP response of such a CONNECT request
surfaced a problem with our HTTP parser, because it doesn't have neither
any headers nor a message body. The parser expected a 'Content-Length'
header to be set in every HTTP response. We fix this by assuming that
a HTTP response doesn't have a message body when the 'Content-Length'
header isn't set.


Diffs (updated)
-

  3rdparty/libprocess/src/decoder.hpp 4c779d42548958e610142438a57529ccb4478053 
  3rdparty/libprocess/src/tests/decoder_tests.cpp 
87563f4fcdfaa2a33c4482533ddff24e062e603a 

Diff: https://reviews.apache.org/r/55496/diff/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

2017-01-13 Thread Benjamin Bannier

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




3rdparty/libprocess/src/decoder.hpp (line 416)


If you made `encoding` an `Option` and dropped the `getOrElse` 
unwrapping, you could directly compare it against `Some("chunked")` below.


- Benjamin Bannier


On Jan. 13, 2017, 2:48 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55496/
> ---
> 
> (Updated Jan. 13, 2017, 2:48 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-6010
> https://issues.apache.org/jira/browse/MESOS-6010
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When behind a proxy, access to websites using SSL can be done by using
> HTTP CONNECT tunneling. This is, for example, used when we pull images
> from a Docker repository. The HTTP response of such a CONNECT request
> surfaced a problem with our HTTP parser, because it doesn't have neither
> any headers nor a message body. The parser expected a 'Content-Length'
> header to be set in every HTTP response. We fix this by assuming that
> a HTTP response doesn't have a message body when the 'Content-Length'
> header isn't set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 4c779d42548958e610142438a57529ccb4478053 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 
> 87563f4fcdfaa2a33c4482533ddff24e062e603a 
> 
> Diff: https://reviews.apache.org/r/55496/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>



Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

2017-01-13 Thread Jan Schlicht

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

(Updated Jan. 13, 2017, 2:48 p.m.)


Review request for mesos, Benjamin Bannier, Gilbert Song, and Jie Yu.


Changes
---

Fixed problems with chunked transfer encodings.


Bugs: MESOS-6010
https://issues.apache.org/jira/browse/MESOS-6010


Repository: mesos


Description
---

When behind a proxy, access to websites using SSL can be done by using
HTTP CONNECT tunneling. This is, for example, used when we pull images
from a Docker repository. The HTTP response of such a CONNECT request
surfaced a problem with our HTTP parser, because it doesn't have neither
any headers nor a message body. The parser expected a 'Content-Length'
header to be set in every HTTP response. We fix this by assuming that
a HTTP response doesn't have a message body when the 'Content-Length'
header isn't set.


Diffs (updated)
-

  3rdparty/libprocess/src/decoder.hpp 4c779d42548958e610142438a57529ccb4478053 
  3rdparty/libprocess/src/tests/decoder_tests.cpp 
87563f4fcdfaa2a33c4482533ddff24e062e603a 

Diff: https://reviews.apache.org/r/55496/diff/


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 55496: Fixed parsing of proxy CONNECT responses.

2017-01-13 Thread Jan Schlicht

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



Don't merge yet, this seems to break chunked transfers.

- Jan Schlicht


On Jan. 13, 2017, 12:28 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55496/
> ---
> 
> (Updated Jan. 13, 2017, 12:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-6010
> https://issues.apache.org/jira/browse/MESOS-6010
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When behind a proxy, access to websites using SSL can be done by using
> HTTP CONNECT tunneling. This is, for example, used when we pull images
> from a Docker repository. The HTTP response of such a CONNECT request
> surfaced a problem with our HTTP parser, because it doesn't have neither
> any headers nor a message body. The parser expected a 'Content-Length'
> header to be set in every HTTP response. We fix this by assuming that
> a HTTP response doesn't have a message body when the 'Content-Length'
> header isn't set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 4c779d42548958e610142438a57529ccb4478053 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 
> 87563f4fcdfaa2a33c4482533ddff24e062e603a 
> 
> Diff: https://reviews.apache.org/r/55496/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>