mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 56116: Implemented new http::Headers abstraction for WWW-Authenticate.
Date Wed, 01 Feb 2017 00:15:13 GMT

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




3rdparty/libprocess/include/process/http.hpp (line 435)
<https://reviews.apache.org/r/56116/#comment235227>

    Let's add a TODO here. According to RFC, an WWWAuthenticate header can contain multiple
"challenges" each of which is in the format of auth-scheme + auth-params.
    
    Here, we assume there is only one auth challenge



3rdparty/libprocess/include/process/http.hpp (lines 435 - 436)
<https://reviews.apache.org/r/56116/#comment235228>

    I would use authScheme and authParam here. You can rename the field members to `authScheme_`
and `authParam_`



3rdparty/libprocess/src/http.cpp (line 602)
<https://reviews.apache.org/r/56116/#comment235244>

    Can we have unit test for this?



3rdparty/libprocess/src/http.cpp (line 612)
<https://reviews.apache.org/r/56116/#comment235245>

    Why tokenize here, that means `,,` is valid? Can you add a unit test for this case?



3rdparty/libprocess/src/http.cpp (line 624)
<https://reviews.apache.org/r/56116/#comment235246>

    RFC says that realm is defined for all auth schemes, but didn't say that each auth challenge
needs to have that.


- Jie Yu


On Jan. 31, 2017, 11:28 p.m., Gilbert Song wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56116/
> -----------------------------------------------------------
> 
> (Updated Jan. 31, 2017, 11:28 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, Shuai Lin, and
Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch introduced a new abstraction for http::Headers and helper
> methods for the http::Headers hashmap, as well as a new
> http::WWWAuthenticate class for the new `Headers` abstraction.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/http.hpp 4b11e085c3abe8d656b0e358b1d3e7bdccae9177

>   3rdparty/libprocess/src/http.cpp 4fd62c8372149a426eb4a70fbc28e423c5962820 
> 
> Diff: https://reviews.apache.org/r/56116/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message