> On Dec. 19, 2017, 1:08 a.m., Benjamin Mahler wrote: > > 3rdparty/libprocess/src/process.cpp > > Lines 3530-3531 (patched) > > <https://reviews.apache.org/r/64574/diff/1/?file=1915259#file1915259line3531> > > > > This approach doesn't seem quite right to me, since it prevents the > > user of libprocess from seeing what the original path was. Have you > > considered any other approaches? (also I think you only needed to trim from > > the tail?) > > Alexander Rukletsov wrote: > Why so? `name` is a local variable based on request path and used to find > appropriate handler. A user of libprocess can still access > `request->url.path`. > > I do trim from the tail only (see two lines above). > > I did consider returning 301 with a trimmed path, but decided not to > since I'm not sure we're allowed to return 301 codes in response to operator > API endpoints.
Ah yes, I see that it's a local variable now, sorry about that! A couple of questions: (1) What happens on the `route()` side? Can a caller setup two routes for "path" and "path/"? And if so, what happens when requests come in for "path" and "path/"? (2) Maybe we could preserve the original trimming of prefix (since it belongs with the removal of the "ID" comment), and instead add the suffix stripping along with a comment about the overall idea here? As it stands, I think the reader will have a hard time realizing that we've decided to treat trailing slash paths the same as non-trailing slash paths? - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64574/#review194106 ----------------------------------------------------------- On Dec. 13, 2017, 1:39 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64574/ > ----------------------------------------------------------- > > (Updated Dec. 13, 2017, 1:39 p.m.) > > > Review request for mesos, Benjamin Bannier and Benjamin Mahler. > > > Bugs: MESOS-5333 > https://issues.apache.org/jira/browse/MESOS-5333 > > > Repository: mesos > > > Description > ------- > > Prior to this patch, adding a trailing '/' to a valid URL path, e.g., > "/state/", yielded a 404 response. This patch ensures that two URLs > which differ only in trailing '/' produce the same result. > > > Diffs > ----- > > 3rdparty/libprocess/src/process.cpp > 75cf1d3b6d3d257ba9bc81c68017a74a6511cebf > 3rdparty/libprocess/src/tests/http_tests.cpp > 9daac715f0242921b7f9f5c20b3eb27f1be802d4 > > > Diff: https://reviews.apache.org/r/64574/diff/1/ > > > Testing > ------- > > Ensured the modified test fails without the fix. > `make check` on Mac OS 10.11.6 and several Linux distributions. > > > Thanks, > > Alexander Rukletsov > >
