You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jan Schlicht <ja...@mesosphere.io> on 2017/01/17 14:35:06 UTC
Re: Review Request 55496: Added support for HTTP responses with
unspecified length.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55496/
-----------------------------------------------------------
(Updated Jan. 17, 2017, 3:35 p.m.)
Review request for mesos, Benjamin Bannier, Gilbert Song, and Jie Yu.
Changes
-------
Chose a different approach, proxied responses are now handled at the caller, as Jie suggested.
Summary (updated)
-----------------
Added support for HTTP responses with unspecified length.
Bugs: MESOS-6010
https://issues.apache.org/jira/browse/MESOS-6010
Repository: mesos
Description (updated)
-------
Some HTTP responses might not set the 'Content-Length' header. This
usually means that the message body should be read until EOF. This
will be done by an additional call of 'decoder.decode' with an empty
string.
Diffs (updated)
-----
3rdparty/libprocess/src/http.cpp 8895f0cfadf1cca9714fc7110ed0914e3db18983
Diff: https://reviews.apache.org/r/55496/diff/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 55496: Added support for HTTP responses with
unspecified length.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55496/#review161870
-----------------------------------------------------------
Fix it, then Ship it!
Let's add a unit test in `3rdparty/libprocess/src/tests/decoder_tests.cpp`.
3rdparty/libprocess/src/http.cpp (lines 767 - 770)
<https://reviews.apache.org/r/55496/#comment233149>
In fact, I would always feed an EOF to the decoder, even if `responses` is non empty:
```
vector<Response> result;
auto appendResult = [=](const dequeue<http::Response*>& responses) mutable {
foreach (Response* response, responses) {
result.push_back(*response);
delete response;
}
};
appendResult(decoder.decode(s.data(), s.length()));
appendResult(decoder.decode("", 0));
if (decoder.failed()) {
return Error("Decoding failed");
}
if (responses.empty()) {
return Error("No response decoded");
}
return result;
```
- Jie Yu
On Jan. 17, 2017, 2:35 p.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55496/
> -----------------------------------------------------------
>
> (Updated Jan. 17, 2017, 2:35 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
> -------
>
> Some HTTP responses might not set the 'Content-Length' header. This
> usually means that the message body should be read until EOF. This
> will be done by an additional call of 'decoder.decode' with an empty
> string.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/http.cpp 8895f0cfadf1cca9714fc7110ed0914e3db18983
>
> Diff: https://reviews.apache.org/r/55496/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 55496: Added support for HTTP responses with
unspecified length.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55496/
-----------------------------------------------------------
(Updated Jan. 18, 2017, 2:44 p.m.)
Review request for mesos, Benjamin Bannier, Gilbert Song, and Jie Yu.
Changes
-------
Added a unit test and addressed issues.
Bugs: MESOS-6010
https://issues.apache.org/jira/browse/MESOS-6010
Repository: mesos
Description
-------
Some HTTP responses might not set the 'Content-Length' header. This
usually means that the message body should be read until EOF. This
will be done by an additional call of 'decoder.decode' with an empty
string.
Diffs (updated)
-----
3rdparty/libprocess/src/http.cpp e2e9b8f7563f8756ee406fecca5900d644834974
3rdparty/libprocess/src/tests/decoder_tests.cpp 87563f4fcdfaa2a33c4482533ddff24e062e603a
Diff: https://reviews.apache.org/r/55496/diff/
Testing
-------
make check
Thanks,
Jan Schlicht