subject:"Review Request 38094\: Added implementation of Http Basic authentication scheme."

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2016-01-04 Thread Ben Mahler

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

Ship it!

Looks good, just some minor items below that I'll address before committing. 
Notably there was a redundant if condition left after the change to use 
.get()'s Option.

3rdparty/libprocess/include/process/authenticator.hpp (line 91)

Not necessary, but should we put the virtual here?

3rdparty/libprocess/src/authenticator.cpp (line 22)

Where did this come from?

3rdparty/libprocess/src/authenticator.cpp (lines 34 - 36)

We're already in the http namespace, did you need these?

3rdparty/libprocess/src/authenticator.cpp (lines 71 - 73)

This case is redundant with your credentials.isNone() check below?

3rdparty/libprocess/src/authenticator.cpp (line 76)

There is a newline here between the variable declaration and the 'if' 
statement, but there isn't in two cases below, then the last case has a 
newline. How about we just use a newline in each case here?

3rdparty/libprocess/src/authenticator.cpp (line 82)

We should probably error out here if the size is not exactly 2, much like 
you did below for the credentials.

3rdparty/libprocess/src/authenticator.cpp (line 96)

Seems a bit odd to put a body here, but not in the other cases, no?

- Ben Mahler

On Jan. 4, 2016, 11:26 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Jan. 4, 2016, 11:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6749fd40e672c3626f6605756bd787a878271034 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> ec7b4aa90bf7024469bf9774e01e46c9f7fd094f 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-29 Thread Ben Mahler


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


The tests look great! Just a minor comment about Forbidden (sorry I sent you 
down that path). Should be good to commit after another iteration.


3rdparty/libprocess/src/authenticator.cpp (lines 53 - 54)


Are these const?



3rdparty/libprocess/src/authenticator.cpp (lines 58 - 84)


These should go at the bottom, under the implementation of 
BasicAuthenticatorProcess::authenticate.



3rdparty/libprocess/src/authenticator.cpp (line 104)


Could we avoid the .get() here and just store the option?

```
Option authorization = request.headers.get("Authorization");

if (authorization.isNone()) {
  return unauthorized;
}

vector components = strings::split(authorization.get(), " ");
```



3rdparty/libprocess/src/authenticator.cpp (lines 121 - 123)


Sorry, looking at this again, looks like this should also be 401 with a 
challenge?

https://tools.ietf.org/html/rfc7235#section-3.1

```
   If the request included authentication credentials, then the 401
   response indicates that authorization has been refused for those
   credentials.
```

Looks like 403 is the real "unauthorized", in the sense that if the request 
was authenticated but the user is not authorized to access to the resource, 
then 403 should be returned: https://tools.ietf.org/html/rfc7231#section-6.5.3

So, `AuthenticationResult.forbidden` should be set only when no-one is 
allowed and therefore we'll never issue a challenge? Perhaps we should clarify 
this in the documentation of AuthenticationResult?



3rdparty/libprocess/src/tests/http_tests.cpp (line 1389)


One more newline here



3rdparty/libprocess/src/tests/http_tests.cpp (lines 1390 - 1446)


Very nice!


- Ben Mahler


On Dec. 14, 2015, 12:53 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 14, 2015, 12:53 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 82778bbd34cfcf54b4268e019d634eb64e506d55 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 9fe27039416290296e43c6327c85721342d02cb9 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-14 Thread Alexander Rojas


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

(Updated Dec. 14, 2015, 1:53 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Review update.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 82778bbd34cfcf54b4268e019d634eb64e506d55 
  3rdparty/libprocess/include/process/authenticator.hpp 
5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
  3rdparty/libprocess/src/CMakeLists.txt 
681f0cfec57e152568da41698c8bdd52c05f65a6 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
9fe27039416290296e43c6327c85721342d02cb9 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-11 Thread Alexander Rojas


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

(Updated Dec. 11, 2015, 3:01 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Fixes indentation.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 82778bbd34cfcf54b4268e019d634eb64e506d55 
  3rdparty/libprocess/include/process/authenticator.hpp 
5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
  3rdparty/libprocess/src/CMakeLists.txt 
681f0cfec57e152568da41698c8bdd52c05f65a6 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
2de75ca1c7e224c36b534c368e7379dc158aa5bb 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-11 Thread Bernd Mathiske


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

Ship it!



3rdparty/libprocess/src/authenticator.cpp (line 49)


indent 2


- Bernd Mathiske


On Dec. 10, 2015, 6:28 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 10, 2015, 6:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 82778bbd34cfcf54b4268e019d634eb64e506d55 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-11 Thread Ben Mahler

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

Thanks for the test, main question is why BasicAuthenticator is not a Process? 
Was that an oversight, or were you intending for it to be pure-const after 
construction and therefore thread-safe?

3rdparty/libprocess/include/process/authenticator.hpp (lines 82 - 97)

This is going to be accessed concurrently, so shouldn't this be a Process? 
Especially since you've marked 'authenticate' as non-const? (I think it could 
have been const)

3rdparty/libprocess/src/authenticator.cpp (lines 47 - 49)

Renaming 'result' to 'unauthorized' will look a bit clearer in the returns 
below. What about Forbidden here? If the credentials are bad, why do we 
re-issue the same challenge?

3rdparty/libprocess/src/authenticator.cpp (lines 57 - 59)

Isn't the empty case caught below when you're parsing with the split?

3rdparty/libprocess/src/authenticator.cpp (lines 79 - 82)

Rather than re-using the object by setting unauthorized to none here, let's 
just use a different object here to keep it simple.

3rdparty/libprocess/src/tests/http_tests.cpp (lines 1387 - 1389)

How about:

// Tests the "Basic" authenticator.

Not sure we need to say the rest?

3rdparty/libprocess/src/tests/http_tests.cpp (line 1390)

Why repeat Authentication here? s/AuthenticationBasic/Basic/ will do

3rdparty/libprocess/src/tests/http_tests.cpp (line 1394)

How about: s/testuser/user/

Why did you need the 'principal' variable here? Looks like you don't use 
it, also we can just hard code "user" and "password" to keep the test really 
simple.

3rdparty/libprocess/src/tests/http_tests.cpp (line 1395)

Why the temporary Owned variable here?

3rdparty/libprocess/src/tests/http_tests.cpp (line 1397)

Can you rebase? I did a s/TEST_AUTHENTICATION_REALM/"realm"/ in your 
previous patch

3rdparty/libprocess/src/tests/http_tests.cpp (line 1398)

How about: s/testpassword/password/

3rdparty/libprocess/src/tests/http_tests.cpp (line 1416)

What's a "pass"? ;)

- Ben Mahler

On Dec. 11, 2015, 2:01 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 11, 2015, 2:01 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 82778bbd34cfcf54b4268e019d634eb64e506d55 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-10 Thread Alexander Rojas


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

(Updated Dec. 10, 2015, 3:28 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase changes in previous review.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 82778bbd34cfcf54b4268e019d634eb64e506d55 
  3rdparty/libprocess/include/process/authenticator.hpp 
5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
  3rdparty/libprocess/src/CMakeLists.txt 
681f0cfec57e152568da41698c8bdd52c05f65a6 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
2de75ca1c7e224c36b534c368e7379dc158aa5bb 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-08 Thread Alexander Rojas


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



3rdparty/libprocess/src/tests/http_tests.cpp (line 1408)


Because the previous RR was testing a specific but that raises under 
specifi circumstances, and this just test that authentication works.


- Alexander Rojas


On Dec. 7, 2015, 4:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 4:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Alexander Rojas


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

(Updated Dec. 7, 2015, 3:18 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
  3rdparty/libprocess/include/process/authenticator.hpp 
5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
  3rdparty/libprocess/src/CMakeLists.txt 
681f0cfec57e152568da41698c8bdd52c05f65a6 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
2de75ca1c7e224c36b534c368e7379dc158aa5bb 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Till Toenshoff

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

3rdparty/libprocess/include/process/authenticator.hpp (line 20)

Add `#include ` since you are using it.

3rdparty/libprocess/src/authenticator.cpp (line 1)

License missing - please make sure you include the Apache License Version 
2.0 header;

```
   // Licensed under the Apache License, Version 2.0 (the "License");  
   // you may not use this file except in compliance with the License.  
   // You may obtain a copy of the License at  
   //  
   // http://www.apache.org/licenses/LICENSE-2.0  
   //  
   // Unless required by applicable law or agreed to in writing, software  
   // distributed under the License is distributed on an "AS IS" BASIS,  
   // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
implied.  
   // See the License for the specific language governing permissions and  
   // limitations under the License
```

3rdparty/libprocess/src/authenticator.cpp (line 28)

You had added a `using std::string` already, we can kill those `std::` 
prefixes for references of `string`.

3rdparty/libprocess/src/authenticator.cpp (line 30)

We commonly derive from clang-format in these cases, the brackets move up;

```
BasicAuthenticator::BasicAuthenticator(
const string& realm,
const hashmap& credentials)
  : realm_(realm), credentials_(credentials) {}
```

3rdparty/libprocess/src/authenticator.cpp (lines 51 - 56)

Shall we combine those two clauses just like you did below with the 
credentials check?

```
  if (components.size() < 2 || components[0] != "Basic") {
return result;
  }
```

3rdparty/libprocess/src/tests/http_tests.cpp (line 1407)

We are `using http::Response` already, kill the namespace.

- Till Toenshoff

On Dec. 7, 2015, 2:18 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 2:18 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Alexander Rukletsov

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

3rdparty/libprocess/include/process/authenticator.hpp (line 26)

Why do you need to include `hashset` here? I don't see any uses of it in 
the ".hpp" file.

3rdparty/libprocess/include/process/authenticator.hpp (lines 94 - 95)

Love trailing underscores for class members!

3rdparty/libprocess/src/authenticator.cpp (line 1)

Do you need a license here?

3rdparty/libprocess/src/authenticator.cpp (lines 25 - 26)

I think `std::*`s go first.

3rdparty/libprocess/src/authenticator.cpp (line 30)

You can remove `std::` prefixes.

3rdparty/libprocess/src/authenticator.cpp (lines 39 - 40)

I see you use the same error message in case something is wrong. Is it done 
on purpose for security reasons? Or do you think it makes sense to extend the 
message with specific note in each case?

3rdparty/libprocess/src/authenticator.cpp (lines 67 - 68)

Let's format this with each condition on a separate line, so it's easier to 
read.

3rdparty/libprocess/src/tests/http_tests.cpp (line 50)

Could you please adjust the order?

3rdparty/libprocess/src/tests/http_tests.cpp (lines 1336 - 1337)

Let's either s/Basic/basic if it's a description, or use the class name 
(`BasicAuthenticator` with backticks) instead.

3rdparty/libprocess/src/tests/http_tests.cpp (lines 1345 - 1346)

const?

3rdparty/libprocess/src/tests/http_tests.cpp (line 1352)

We usually tend to put a verb first, e.g. "Provide no credentials", 
"Provide wrong credentials". Do you think it makes sense to fix it for 
consistency?

3rdparty/libprocess/src/tests/http_tests.cpp (lines 1352 - 1360)

One thing we did in quota tests is we put each scenario in a scope. This 
way it's more clear to the user where are the boundaries of each scenario, and 
also it's more clear that a comment prepending the scope is for the whole scope 
and not for a single following line

3rdparty/libprocess/src/tests/http_tests.cpp (lines 1361 - 1363)

Do you want to add a test case for wrong principal (maybe even with 
"testpassword")?

- Alexander Rukletsov

On Dec. 7, 2015, 2:18 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 2:18 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Alexander Rojas


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

(Updated Dec. 7, 2015, 4:02 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
  3rdparty/libprocess/include/process/authenticator.hpp 
5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
  3rdparty/libprocess/src/CMakeLists.txt 
681f0cfec57e152568da41698c8bdd52c05f65a6 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
2de75ca1c7e224c36b534c368e7379dc158aa5bb 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Alexander Rojas


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

(Updated Dec. 7, 2015, 4:11 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
  3rdparty/libprocess/include/process/authenticator.hpp 
5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
  3rdparty/libprocess/src/CMakeLists.txt 
681f0cfec57e152568da41698c8bdd52c05f65a6 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
2de75ca1c7e224c36b534c368e7379dc158aa5bb 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Till Toenshoff


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

Ship it!


Ship It!

- Till Toenshoff


On Dec. 7, 2015, 3:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Alexander Rojas



> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 25-26
> > 
> >
> > I think `std::*`s go first.

Not according to all the examples I checked, see `process.cpp` or all the 
`main.cpp`


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 39-40
> > 
> >
> > I see you use the same error message in case something is wrong. Is it 
> > done on purpose for security reasons? Or do you think it makes sense to 
> > extend the message with specific note in each case?

It is actually not an error message but the challenge(-s) to be emited to the 
client in authentication fails (See the constructor of 
[Unauthorized](https://github.com/apache/mesos/blob/49296b9a80ec26bf77bc9191fff7b2f5e143b1d2/3rdparty/libprocess/include/process/http.hpp#L521)
 which takes a vector). 

Still, the reason why you don't give detailed error messages is because with 
authentication you want to be quite vague. When you failed to authenticate to a 
site, it tells you that either your username doesn't exist or your password was 
wrong, since you rather don't tell which one of the two failed.


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, line 50
> > 
> >
> > Could you please adjust the order?

Not relevant anymore, since the authentication code got its own namespace.


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, lines 1336-1337
> > 
> >
> > Let's either s/Basic/basic if it's a description, or use the class name 
> > (`BasicAuthenticator` with backticks) instead.

I would rather not, since you find _Basic_ in pretty much all the literature on 
the topic (See [RFC 26217](https://tools.ietf.org/html/rfc2617)).


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, lines 1352-1360
> > 
> >
> > One thing we did in quota tests is we put each scenario in a scope. 
> > This way it's more clear to the user where are the boundaries of each 
> > scenario, and also it's more clear that a comment prepending the scope is 
> > for the whole scope and not for a single following line

Great idea.


> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/tests/http_tests.cpp, line 1352
> > 
> >
> > We usually tend to put a verb first, e.g. "Provide no credentials", 
> > "Provide wrong credentials". Do you think it makes sense to fix it for 
> > consistency?

I didn't even know there was a rule about it (which I find rather extreme). In 
any case, I think putting a verb first violates the _subject-verb-object_ 
syntax of english and makes it sound imperative (which it is not). In this case 
by use of the passive tense I transform the _object_ into the _subject_ and 
still have a correctly formed english sentence.


- Alexander


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


On Dec. 7, 2015, 3:18 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:18 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Alexander Rojas



> On Dec. 7, 2015, 3:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 25-26
> > 
> >
> > I think `std::*`s go first.
> 
> Alexander Rojas wrote:
> Not according to all the examples I checked, see `process.cpp` or all the 
> `main.cpp`
> 
> Alexander Rukletsov wrote:
> Yeah, we are inconsistent, but I think that's where we want to be : ).

Well, after checking more than 20 files taken at random from the list producing 
after grepping for `^using +std::.*;$` we seem to be really consistent about 
the ordering, just `libprocess/src/http.cpp` has the ordering you want.

On top of that, the styleguide says nothing on the topic.


- Alexander


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


On Dec. 7, 2015, 4:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 4:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Alexander Rukletsov



> On Dec. 7, 2015, 2:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 25-26
> > 
> >
> > I think `std::*`s go first.
> 
> Alexander Rojas wrote:
> Not according to all the examples I checked, see `process.cpp` or all the 
> `main.cpp`

Yeah, we are inconsistent, but I think that's where we want to be : ).


- Alexander


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


On Dec. 7, 2015, 3:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Alexander Rukletsov



> On Dec. 7, 2015, 2:22 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/authenticator.cpp, lines 39-40
> > 
> >
> > I see you use the same error message in case something is wrong. Is it 
> > done on purpose for security reasons? Or do you think it makes sense to 
> > extend the message with specific note in each case?
> 
> Alexander Rojas wrote:
> It is actually not an error message but the challenge(-s) to be emited to 
> the client in authentication fails (See the constructor of 
> [Unauthorized](https://github.com/apache/mesos/blob/49296b9a80ec26bf77bc9191fff7b2f5e143b1d2/3rdparty/libprocess/include/process/http.hpp#L521)
>  which takes a vector). 
> 
> Still, the reason why you don't give detailed error messages is because 
> with authentication you want to be quite vague. When you failed to 
> authenticate to a site, it tells you that either your username doesn't exist 
> or your password was wrong, since you rather don't tell which one of the two 
> failed.

Got it, mind adding a comment about this for the next reader?


- Alexander


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


On Dec. 7, 2015, 3:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-12-07 Thread Alexander Rukletsov


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



3rdparty/libprocess/src/tests/http_tests.cpp (line 1408)


Why do you use `http::Connection.send()` in the previous RR and 
`http::get()` here?


- Alexander Rukletsov


On Dec. 7, 2015, 3:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Dec. 7, 2015, 3:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 6ec6d7989df647f466ac6079738835ffcb2ea8ee 
>   3rdparty/libprocess/include/process/authenticator.hpp 
> 5a32e9a38a0bec7aa3faef23b792f3bf3d659d4f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> 681f0cfec57e152568da41698c8bdd52c05f65a6 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 2de75ca1c7e224c36b534c368e7379dc158aa5bb 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-11-17 Thread Alexander Rojas


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

(Updated Nov. 17, 2015, 2:37 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Third round of reviews.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
5e70f1896a86104ac01dfe725eb4d7d1d25bee77 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-11-16 Thread Alexander Rojas


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

(Updated Nov. 16, 2015, 5:26 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Second round of reviews.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
5e70f1896a86104ac01dfe725eb4d7d1d25bee77 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-11-16 Thread Alexander Rojas


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

(Updated Nov. 16, 2015, 10:54 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Update for changes in the interface.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
5e70f1896a86104ac01dfe725eb4d7d1d25bee77 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-11-09 Thread Alexander Rojas


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

(Updated Nov. 10, 2015, 5 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
7eb4ef187b2cb358c370d0381db65b8e18668bab 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-11-09 Thread Alexander Rojas


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

(Updated Nov. 9, 2015, 11:33 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am cdefa37528ea69422978a9772f955042e882dde4 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
7eb4ef187b2cb358c370d0381db65b8e18668bab 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-11-04 Thread Alexander Rojas


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

(Updated Nov. 4, 2015, 12:29 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebasing.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 404a7438e7cd53d9295fe6025b5af5fb83df939b 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
7eb4ef187b2cb358c370d0381db65b8e18668bab 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-10-21 Thread Alexander Rojas


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

(Updated Oct. 21, 2015, 2:37 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Adresses issues from Till's review.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 404a7438e7cd53d9295fe6025b5af5fb83df939b 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-10-20 Thread Till Toenshoff


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



3rdparty/libprocess/src/authenticator.cpp (lines 81 - 84)


Let's stick to non doxygen format style for those.


- Till Toenshoff


On Sept. 30, 2015, 8:21 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Sept. 30, 2015, 8:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fb9bd04832779ac43151f2feb3dfbf58cb996434 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> c380f356548cf9f5491044bccabcd9c66ad5f55a 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-10-20 Thread Alexander Rojas


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

(Updated Oct. 20, 2015, 2:07 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Fixes Till's issue.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am 404a7438e7cd53d9295fe6025b5af5fb83df939b 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d13d3888abbf3db552df4a9f83e54667e598ded9 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-10-20 Thread Till Toenshoff


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


Sorry for not finding the below in the first pass ...


3rdparty/libprocess/src/authenticator.cpp (line 30)


Add blank line.



3rdparty/libprocess/src/authenticator.cpp (line 37)


Add blank line.



3rdparty/libprocess/src/authenticator.cpp (line 78)


Add blank line.



3rdparty/libprocess/src/authenticator.cpp (line 87)


Add blank line.



3rdparty/libprocess/src/authenticator.cpp (line 92)


Add blank line.


- Till Toenshoff


On Oct. 20, 2015, 12:07 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Oct. 20, 2015, 12:07 p.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 404a7438e7cd53d9295fe6025b5af5fb83df939b 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fb9bd04832779ac43151f2feb3dfbf58cb996434 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d13d3888abbf3db552df4a9f83e54667e598ded9 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-09-30 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [37714, 37996, 37997, 37998, 37999, 38000, 38094]

All tests passed.

- Mesos ReviewBot


On Sept. 30, 2015, 8:21 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Sept. 30, 2015, 8:21 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fb9bd04832779ac43151f2feb3dfbf58cb996434 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> c380f356548cf9f5491044bccabcd9c66ad5f55a 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-09-30 Thread Alexander Rojas


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

(Updated Sept. 30, 2015, 10:21 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Rebase.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
c380f356548cf9f5491044bccabcd9c66ad5f55a 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-09-24 Thread Alexander Rojas


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

(Updated Sept. 24, 2015, 12:33 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Added new file to CMakeFiles.txt


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/CMakeLists.txt 
fb9bd04832779ac43151f2feb3dfbf58cb996434 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-09-24 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [37714, 37996, 37997, 37998, 37999, 38000, 38094]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 10:33 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Sept. 24, 2015, 10:33 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/src/CMakeLists.txt 
> fb9bd04832779ac43151f2feb3dfbf58cb996434 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-09-23 Thread Mesos ReviewBot


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


Patch looks great!

Reviews applied: [37714, 37996, 37997, 37998, 37999, 38000, 38094]

All tests passed.

- Mesos ReviewBot


On Sept. 23, 2015, 11:17 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Sept. 23, 2015, 11:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-09-23 Thread Alexander Rojas


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

(Updated Sept. 23, 2015, 1:17 p.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

Forces buildbot to run again after fix in previous patch.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-09-23 Thread Alex Clemmer


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


Can we please add this file to the CMakeLists.txt in 3rdparty/libprocess/src as 
well?

- Alex Clemmer


On Sept. 23, 2015, 11:17 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38094/
> ---
> 
> (Updated Sept. 23, 2015, 11:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
>   3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> d0b9399d38fa284466a012a21080b1d9007af98b 
> 
> Diff: https://reviews.apache.org/r/38094/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38094: Added implementation of Http Basic authentication scheme.

2015-09-23 Thread Alexander Rojas


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

(Updated Sept. 23, 2015, 10:58 a.m.)


Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
Toenshoff.


Changes
---

No longer WIP, opening to the community.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/include/process/authenticator.hpp PRE-CREATION 
  3rdparty/libprocess/src/authenticator.cpp PRE-CREATION 
  3rdparty/libprocess/src/tests/http_tests.cpp 
d0b9399d38fa284466a012a21080b1d9007af98b 

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


Testing
---

make check


Thanks,

Alexander Rojas

37 matches

Mail list logo