Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-10 Thread Jan Schlicht
> On March 8, 2017, 8:43 p.m., Greg Mann wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 168-170 (patched) > > > > > > If you want to improve the encapsulation of the error messages a bit, > > you could prefix

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-09 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review168547 --- Ship it! Ship It! - Vinod Kone On March 9, 2017, 2:47 p.m.,

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-09 Thread Jan Schlicht
> On March 8, 2017, 8:43 p.m., Greg Mann wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 37 (patched) > > > > > > Should the 3 helpers here be either enclosed in an anonymous namespace > > or decorated with

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-09 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/ --- (Updated March 9, 2017, 3:47 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-08 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review168328 --- Fix it, then Ship it! 3rdparty/libprocess/src/jwt.cpp Lines

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-08 Thread Jan Schlicht
> On March 7, 2017, 12:33 a.m., Greg Mann wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 169 (patched) > > > > > > Do you think `parse_component` would be a better name for this? > > Jan Schlicht wrote: >

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-08 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/ --- (Updated March 8, 2017, 3:23 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-08 Thread Jan Schlicht
> On March 7, 2017, 10:37 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/src/tests/jwt_tests.cpp > > Lines 198 (patched) > > > > > > can you do EXPECT_SOME? > > Jan Schlicht wrote: > Unfortunately not, because

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-08 Thread Jan Schlicht
> On March 7, 2017, 10:37 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 194 (patched) > > > > > > s/JOSE/JSON/ ? > > Greg Mann wrote: > Strangely, "JOSE header" just happens to be the

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Greg Mann
> On March 7, 2017, 9:37 p.m., Vinod Kone wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 194 (patched) > > > > > > s/JOSE/JSON/ ? Strangely, "JOSE header" just happens to be the name of the JWT header:

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Greg Mann
> On March 6, 2017, 11:33 p.m., Greg Mann wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 169 (patched) > > > > > > Do you think `parse_component` would be a better name for this? > > Jan Schlicht wrote: >

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Vinod Kone
> On March 6, 2017, 11:33 p.m., Greg Mann wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 169 (patched) > > > > > > Do you think `parse_component` would be a better name for this? > > Jan Schlicht wrote: >

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review168192 --- 3rdparty/libprocess/src/jwt.cpp Lines 120 (patched)

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review168168 --- 3rdparty/libprocess/src/tests/jwt_tests.cpp Lines 105 (patched)

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review168149 --- 3rdparty/libprocess/include/process/jwt.hpp Lines 127-128

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Greg Mann
> On March 6, 2017, 11:33 p.m., Greg Mann wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 169 (patched) > > > > > > Do you think `parse_component` would be a better name for this? > > Jan Schlicht wrote: >

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Greg Mann
> On March 6, 2017, 11:33 p.m., Greg Mann wrote: > > 3rdparty/libprocess/include/process/jwt.hpp > > Lines 42 (patched) > > > > > > Should we also provide a link to RFC-7515 here? > > Jan Schlicht wrote: > IMO

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/ --- (Updated March 7, 2017, 4:57 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Jan Schlicht
> On March 7, 2017, 12:33 a.m., Greg Mann wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 169 (patched) > > > > > > Do you think `parse_component` would be a better name for this? But it's also base64-decoding

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-07 Thread Jan Schlicht
> On March 7, 2017, 12:33 a.m., Greg Mann wrote: > > 3rdparty/libprocess/include/process/jwt.hpp > > Lines 42 (patched) > > > > > > Should we also provide a link to RFC-7515 here? IMO we shouldn't. While RFC 7519

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-06 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review168040 --- 3rdparty/libprocess/include/process/jwt.hpp Lines 13-14

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-06 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/ --- (Updated March 6, 2017, 3:53 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-03 Thread Jan Schlicht
> On March 1, 2017, 8:24 a.m., Greg Mann wrote: > > 3rdparty/libprocess/src/jwt.cpp > > Lines 94-95 (patched) > > > > > > Join on one line: > > > > `} else if ( ... ) {` > > > > Could also use a

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-03-03 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/ --- (Updated March 3, 2017, 3:03 p.m.) Review request for mesos, Alexander Rojas

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-28 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review167186 --- Sorry for not noticing this before: since this is a libprocess

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-23 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/ --- (Updated Feb. 23, 2017, 3:22 p.m.) Review request for mesos and Greg Mann.

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-22 Thread Greg Mann
> On Feb. 20, 2017, 7:36 a.m., Greg Mann wrote: > > Regarding the description: I'm curious how exactly the current > > implementation isn't compliant with RFCs 7515/7519? The one thing I noticed > > was the lack of support for the 'crit' header parameter. > > Jan Schlicht wrote: > There

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-22 Thread Jan Schlicht
> On Feb. 20, 2017, 8:36 a.m., Greg Mann wrote: > > Regarding the description: I'm curious how exactly the current > > implementation isn't compliant with RFCs 7515/7519? The one thing I noticed > > was the lack of support for the 'crit' header parameter. > > Jan Schlicht wrote: > There

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-22 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/ --- (Updated Feb. 22, 2017, 3:26 p.m.) Review request for mesos and Greg Mann.

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-22 Thread Jan Schlicht
> On Feb. 20, 2017, 8:36 a.m., Greg Mann wrote: > > Regarding the description: I'm curious how exactly the current > > implementation isn't compliant with RFCs 7515/7519? The one thing I noticed > > was the lack of support for the 'crit' header parameter. There isn't support for `alg=none`

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-19 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review166057 --- Regarding the description: I'm curious how exactly the current

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-16 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/#review165823 --- Patch looks great! Reviews applied: [56665, 5, 56667]

Re: Review Request 56667: Added support for JSON Web Tokens.

2017-02-16 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56667/ --- (Updated Feb. 16, 2017, 10:35 a.m.) Review request for mesos and Greg Mann.