Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-28 Thread haosdent huang 


> On Sept. 24, 2016, 8:02 p.m., haosdent huang wrote:
> > src/master/http.cpp, lines 2046-2048
> > 
> >
> > Hi, @drcrallen This comment is not true. As 
> > https://github.com/apache/mesos/blob/master/src/master/http.cpp#L427-L429, 
> > we use `::redirect` to redirect any path to the leading master internally. 
> > So the `request.url.path` may be anything here. This is why I suggest to 
> > change to 
> > 
> > ```
> > diff --git a/src/master/http.cpp b/src/master/http.cpp
> > index e9f9d16..c77229a 100644
> > --- a/src/master/http.cpp
> > +++ b/src/master/http.cpp
> > @@ -2038,6 +2038,11 @@ Future Master::Http::redirect(const 
> > Request& request) const
> >  // When request url is '/redirect' or '/master/redirect', redirect 
> > to the
> >  // base url of leading master to avoid infinite redirect loop.
> >  return TemporaryRedirect(basePath);
> > +  } else if (strings::startsWith(request.url.path, "/redirect/") ||
> > + strings::startsWith(request.url.path,
> > + "/" + master->self().id + 
> > "/redirect/")) {
> > +// Stop redirection here to avoid fall into an infinite 
> > redirection loop.
> > +return NotFound();
> >} else {
> > ```
> > 
> > in the previous comment.
> 
> Charles Allen wrote:
> Sure, I can do that.

Many thanks!


- haosdent


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


On Sept. 23, 2016, 6:04 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 23, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:17 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:19 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:21 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect_foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:23 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:49 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:50 GMT
> < Location: //10.17.97.185:5050
> < Conten

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-28 Thread Charles Allen 


> On Sept. 24, 2016, 8:02 p.m., haosdent huang wrote:
> > src/master/http.cpp, lines 2046-2048
> > 
> >
> > Hi, @drcrallen This comment is not true. As 
> > https://github.com/apache/mesos/blob/master/src/master/http.cpp#L427-L429, 
> > we use `::redirect` to redirect any path to the leading master internally. 
> > So the `request.url.path` may be anything here. This is why I suggest to 
> > change to 
> > 
> > ```
> > diff --git a/src/master/http.cpp b/src/master/http.cpp
> > index e9f9d16..c77229a 100644
> > --- a/src/master/http.cpp
> > +++ b/src/master/http.cpp
> > @@ -2038,6 +2038,11 @@ Future Master::Http::redirect(const 
> > Request& request) const
> >  // When request url is '/redirect' or '/master/redirect', redirect 
> > to the
> >  // base url of leading master to avoid infinite redirect loop.
> >  return TemporaryRedirect(basePath);
> > +  } else if (strings::startsWith(request.url.path, "/redirect/") ||
> > + strings::startsWith(request.url.path,
> > + "/" + master->self().id + 
> > "/redirect/")) {
> > +// Stop redirection here to avoid fall into an infinite 
> > redirection loop.
> > +return NotFound();
> >} else {
> > ```
> > 
> > in the previous comment.

Sure, I can do that.


- Charles


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


On Sept. 23, 2016, 6:04 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 23, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:17 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:19 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:21 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect_foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:23 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:49 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:50 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-24 Thread haosdent huang 

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



It would be better to adjust the commit message like

```
Avoided the redirection loop when request `/redirect/xxx`.

Avoid the redirection loop when send `/redirect/xxx` request to the 
master.
```

- haosdent huang


On Sept. 23, 2016, 6:04 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 23, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:17 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:19 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:21 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect_foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:23 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:49 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:50 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect_foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:53 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:55 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> ```
> 
> It is worth noting that in this PR /master/redirect/  and /redirect/ return 
> 404, not 307. This behavior is consistent with master.
> 
> 
> Thanks,
> 
> Charles Allen
> 
>

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-24 Thread haosdent huang 

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




src/master/http.cpp (lines 2046 - 2048)


Hi, @drcrallen This comment is not true. As 
https://github.com/apache/mesos/blob/master/src/master/http.cpp#L427-L429, we 
use `::redirect` to redirect any path to the leading master internally. So the 
`request.url.path` may be anything here. This is why I suggest to change to 

```
diff --git a/src/master/http.cpp b/src/master/http.cpp
index e9f9d16..c77229a 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2038,6 +2038,11 @@ Future Master::Http::redirect(const 
Request& request) const
 // When request url is '/redirect' or '/master/redirect', redirect to 
the
 // base url of leading master to avoid infinite redirect loop.
 return TemporaryRedirect(basePath);
+  } else if (strings::startsWith(request.url.path, "/redirect/") ||
+ strings::startsWith(request.url.path,
+ "/" + master->self().id + "/redirect/")) {
+// Stop redirection here to avoid fall into an infinite redirection 
loop.
+return NotFound();
   } else {
```

in the previous comment.


- haosdent huang


On Sept. 23, 2016, 6:04 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 23, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:17 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:19 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:21 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect_foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:23 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:49 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:50 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect_f

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-23 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [52105]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 23, 2016, 6:04 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 23, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:17 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:19 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:21 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect_foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:23 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:49 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:50 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect_foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:53 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:55 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> ```
> 
> It is worth noting that in this PR /master/redirect/  and /redirect/ return 
> 404, not 307. This behavior is consistent with master.
> 
> 
> Thanks,
> 
> Charles Allen
> 
>

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-23 Thread Charles Allen 


> On Sept. 23, 2016, 3:05 p.m., haosdent huang wrote:
> > src/master/http.cpp, lines 2038-2040
> > 
> >
> > Another problem here is if we call `redirect(/redirect_a)` in http.cpp, 
> > would get unexpected result. So how about just return 404 which consistent 
> > with `/redirect/`
> > 
> > ```
> >   // When current master is not the leader, redirect to the leading 
> > master.
> >   if (!master->elected()) {
> > return redirect(request);
> >   }
> > ```
> 
> Charles Allen wrote:
> Would that mean things like `/redirect/foo` should also return 404?
> 
> haosdent huang wrote:
> yes.
> 
> haosdent huang wrote:
> This is consistent with `/redirect/`. And I think something like 
> `/redirect/foo/redirect/bar/xxx` make it messy if we keep redirecting to 
> `/foo/redirect/xxx` here.

Please see updated examples. Pretty much everything is 404 except explicitly 
`/redirect` and `/{master_id}/redirect`


- Charles


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


On Sept. 23, 2016, 6:04 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 23, 2016, 6:04 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:17 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:19 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:21 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect_foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:23 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:49 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Fri, 23 Sep 2016 17:30:50 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect_foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect_foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Fri, 23 Sep 2016 17:30:53 GMT
> < Content-Length: 0
> <
> * Connect

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-23 Thread Charles Allen 

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

(Updated Sept. 23, 2016, 6:04 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Fixing review comments


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


Repository: mesos


Description
---

Prevent `/redirect/foo` loop.


Diffs (updated)
-

  src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 

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


Testing (updated)
---

Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`


```
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Fri, 23 Sep 2016 17:30:17 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect/ HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Fri, 23 Sep 2016 17:30:19 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/foo
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect/foo HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Fri, 23 Sep 2016 17:30:21 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect_foo
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect_foo HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Fri, 23 Sep 2016 17:30:23 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/ HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Fri, 23 Sep 2016 17:30:49 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Fri, 23 Sep 2016 17:30:50 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect_foo
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect_foo HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Fri, 23 Sep 2016 17:30:53 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/foo
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/foo HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Fri, 23 Sep 2016 17:30:55 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
```

It is worth noting that in this PR /master/redirect/  and /redirect/ return 
404, not 307. This behavior is consistent with master.


Thanks,

Charles Allen

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-23 Thread haosdent huang 


> On Sept. 23, 2016, 3:05 p.m., haosdent huang wrote:
> > src/master/http.cpp, lines 2038-2040
> > 
> >
> > Another problem here is if we call `redirect(/redirect_a)` in http.cpp, 
> > would get unexpected result. So how about just return 404 which consistent 
> > with `/redirect/`
> > 
> > ```
> >   // When current master is not the leader, redirect to the leading 
> > master.
> >   if (!master->elected()) {
> > return redirect(request);
> >   }
> > ```
> 
> Charles Allen wrote:
> Would that mean things like `/redirect/foo` should also return 404?
> 
> haosdent huang wrote:
> yes.

This is consistent with `/redirect/`. And I think something like 
`/redirect/foo/redirect/bar/xxx` make it messy if we keep redirecting to 
`/foo/redirect/xxx` here.


- haosdent


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


On Sept. 22, 2016, 8:47 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 22, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:18 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:23 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:29 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:31 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:34 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:41 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirectNOTFOUND/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirectNOTFOUND/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:47 GMT

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-23 Thread haosdent huang 


> On Sept. 23, 2016, 3:05 p.m., haosdent huang wrote:
> > src/master/http.cpp, lines 2038-2040
> > 
> >
> > Another problem here is if we call `redirect(/redirect_a)` in http.cpp, 
> > would get unexpected result. So how about just return 404 which consistent 
> > with `/redirect/`
> > 
> > ```
> >   // When current master is not the leader, redirect to the leading 
> > master.
> >   if (!master->elected()) {
> > return redirect(request);
> >   }
> > ```
> 
> Charles Allen wrote:
> Would that mean things like `/redirect/foo` should also return 404?

yes.


- haosdent


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


On Sept. 22, 2016, 8:47 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 22, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:18 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:23 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:29 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:31 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:34 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:41 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirectNOTFOUND/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirectNOTFOUND/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:47 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> ```
> 
> It is worth noting that in this PR /master/redirect/  and /redirect/ return 
> 404, not 307. This behavior is con

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-23 Thread Charles Allen 


> On Sept. 23, 2016, 3:05 p.m., haosdent huang wrote:
> > src/master/http.cpp, lines 2038-2040
> > 
> >
> > Another problem here is if we call `redirect(/redirect_a)` in http.cpp, 
> > would get unexpected result. So how about just return 404 which consistent 
> > with `/redirect/`
> > 
> > ```
> >   // When current master is not the leader, redirect to the leading 
> > master.
> >   if (!master->elected()) {
> > return redirect(request);
> >   }
> > ```

Would that mean things like `/redirect/foo` should also return 404?


- Charles


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


On Sept. 22, 2016, 8:47 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 22, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:18 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:23 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:29 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:31 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:34 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:41 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirectNOTFOUND/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirectNOTFOUND/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:47 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> ```
> 
> It is worth noting that in this PR /master/redirect/  and /redirect/ return 
> 404, not 307. This behavior is consistent with master.
> 
> 
> Thanks,
>

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-23 Thread haosdent huang 

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




src/master/http.cpp (lines 2036 - 2044)


Becasue we have an util function `startsWith`, how about change it like

```
diff --git a/src/master/http.cpp b/src/master/http.cpp
index e9f9d16..c77229a 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -2038,6 +2038,11 @@ Future Master::Http::redirect(const 
Request& request) const
 // When request url is '/redirect' or '/master/redirect', redirect to 
the
 // base url of leading master to avoid infinite redirect loop.
 return TemporaryRedirect(basePath);
+  } else if (strings::startsWith(request.url.path, "/redirect/") ||
+ strings::startsWith(request.url.path,
+ "/" + master->self().id + "/redirect/")) {
+// Stop redirection here to avoid fall into an infinite redirection 
loop.
+return NotFound();
   } else {
```



src/master/http.cpp (lines 2038 - 2040)


Another problem here is if we call `redirect(/redirect_a)` in http.cpp, 
would get unexpected result. So how about just return 404 which consistent with 
`/redirect/`

```
  // When current master is not the leader, redirect to the leading master.
  if (!master->elected()) {
return redirect(request);
  }
```


- haosdent huang


On Sept. 22, 2016, 8:47 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 22, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:18 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:23 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:29 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:31 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:34 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
>

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-22 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [52105]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Sept. 22, 2016, 8:47 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 22, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:18 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:23 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:29 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:31 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:34 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirect/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:41 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/redirectNOTFOUND/foo/bar
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirectNOTFOUND/foo/bar HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:47 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> ```
> 
> It is worth noting that in this PR /master/redirect/  and /redirect/ return 
> 404, not 307. This behavior is consistent with master.
> 
> 
> Thanks,
> 
> Charles Allen
> 
>

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-22 Thread Charles Allen 


> On Sept. 21, 2016, 3:18 p.m., haosdent huang wrote:
> > Hi, @drcrallen Thanks a lot for your patch!
> > 
> > I saw your patch try to redirect 
> > `http://SOME_MASTER:5050/master/redirect/master/frameworks` to 
> > `http://LEADER_MASTER:5050/master/frameworks`. Could you elaborate the user 
> > cases about this? Because we have handled the endpoint redirection 
> > automatically internal. For example, when you request 
> > `http://NON_LEADING_MASTER:5050/master/frameworks`, mesos master would 
> > redirect to `http://LEADING_MASTER:5050/master/frameworks`.
> 
> Charles Allen wrote:
> That comes from two things:
> 
> 1. 
> http://mesos.apache.org/documentation/latest/endpoints/master/redirect/ 
> suggests using redirect as a UI bookmarking mechanism, as such, it seemed 
> reasonable for other calls to be able to redirect through the same mechanism. 
> This might be a left-over suggestion from pre-1.0 though. An alternative 
> thing here would be to update the redirect docs to remove the suggestion for 
> UI bookmarking if it is no longer required.
> 2. That's how some other internal discovery mechanism are setup here, so 
> I kind of expected http://SOME_MASTER:5050/master/redirect/state to redirect 
> to the master and get the cluster state. But if the non-leading masters 
> should redirect to the leading masters then I don't think that is helpful 
> anymore, and I think that behavior changed with 1.0?
> 
> haosdent huang wrote:
> Hi, @drcrallen
> 
> >1. ... suggests using redirect as a UI bookmarking mechanism
> 
> I think you may misunderstand this. We suggest to bookmark 
> `http://SOME_MASTER:5050/master/redirect` to open the leading master quickly. 
> And it has never work for `/master/redirect/xxx` before.
> 
> > 2. xxx so I kind of expected 
> http://SOME_MASTER:5050/master/redirect/state to redirect to the master and 
> get the cluster state.
> 
> I think just need to access `http://SOME_MASTER:5050/master/state` here. 
> If `SOME_MASTER` is not the leading master, it would redirct to 
> `http://LEADING_MASTER/master/state` automatically. This hehaviour never 
> change with 1.0
> 
> Charles Allen wrote:
> Allright, I'll simplify the patch to only do the string starts with check.

@Haosdent simplified scope, and updated master comment testing info with new 
results.


- Charles


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


On Sept. 22, 2016, 8:47 p.m., Charles Allen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52105/
> ---
> 
> (Updated Sept. 22, 2016, 8:47 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6210
> https://issues.apache.org/jira/browse/MESOS-6210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Prevent `/redirect/foo` loop.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 
> 
> Diff: https://reviews.apache.org/r/52105/diff/
> 
> 
> Testing
> ---
> 
> Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`
> 
> 
> ```
> Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:18 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 307 Temporary Redirect
> < Date: Thu, 22 Sep 2016 20:35:23 GMT
> < Location: //10.17.97.185:5050
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/ HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > Accept: */*
> >
> < HTTP/1.1 404 Not Found
> < Date: Thu, 22 Sep 2016 20:35:29 GMT
> < Content-Length: 0
> <
> * Connection #0 to host 127.0.0.1 left intact
> Charless-MacBook-Pro:~ charlesallen$ curl -v 
> http://127.0.0.1:5050/master/redirect/foo
> *   Trying 127.0.0.1...
> * Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> > GET /master/redirect/foo HTTP/1.1
> > Host: 127.0.0.1:5050
> > User-Agent: curl/7.43.0
> > A

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-22 Thread Charles Allen 

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

(Updated Sept. 22, 2016, 8:47 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated test case


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


Repository: mesos


Description
---

Prevent `/redirect/foo` loop.


Diffs
-

  src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 

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


Testing (updated)
---

Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`


```
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:18 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:23 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/ HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Thu, 22 Sep 2016 20:35:29 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/foo
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/foo HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:31 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/foo/bar
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/foo/bar HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:34 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/redirect/foo/bar
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect/foo/bar HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Thu, 22 Sep 2016 20:35:41 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/redirectNOTFOUND/foo/bar
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirectNOTFOUND/foo/bar HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Thu, 22 Sep 2016 20:35:47 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
```

It is worth noting that in this PR /master/redirect/  and /redirect/ return 
404, not 307. This behavior is consistent with master.


Thanks,

Charles Allen

Re: Review Request 52105: Prevent `/redirect/foo` loop.

2016-09-22 Thread Charles Allen 

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

(Updated Sept. 22, 2016, 8:46 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Now with simple loop prevention


Summary (updated)
-

Prevent `/redirect/foo` loop.


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


Repository: mesos


Description (updated)
---

Prevent `/redirect/foo` loop.


Diffs (updated)
-

  src/master/http.cpp 9005e7c308d5f57c6f5c573951d468a3ba730740 

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


Testing
---

Single Node started with `./src/mesos-master --work_dir=/tmp/mesos`


```
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Tue, 20 Sep 2016 23:12:24 GMT
< Location: //10.17.97.185:5050
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v http://127.0.0.1:5050/redirect/foo
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /redirect/foo HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Tue, 20 Sep 2016 23:12:35 GMT
< Location: //10.17.97.185:5050/foo
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/test
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/test HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Tue, 20 Sep 2016 23:12:49 GMT
< Location: //10.17.97.185:5050/test
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirectNOTFOUND/test
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirectNOTFOUND/test HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Tue, 20 Sep 2016 23:12:57 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/ HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Date: Tue, 20 Sep 2016 23:13:32 GMT
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
Charless-MacBook-Pro:~ charlesallen$ curl -v 
http://127.0.0.1:5050/master/redirect/test
*   Trying 127.0.0.1...
* Connected to 127.0.0.1 (127.0.0.1) port 5050 (#0)
> GET /master/redirect/test HTTP/1.1
> Host: 127.0.0.1:5050
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 307 Temporary Redirect
< Date: Tue, 20 Sep 2016 23:13:43 GMT
< Location: //10.17.97.185:5050/test
< Content-Length: 0
<
* Connection #0 to host 127.0.0.1 left intact
```

It is worth noting that in this PR /master/redirect/  and /redirect/ return 
404, not 307. This behavior is consistent with master.


Thanks,

Charles Allen