Skip to content

Commit

Permalink
Fix inconsistencies with URI::Path
Browse files Browse the repository at this point in the history 
URI paths are now defined as "there is a slash between each segment".
So, "http://localhost/" ends up with two path segments, both empty.
In practice, this means a trailing slash always has a trailing empty
element, and a leading slash (an absolute path) always has a leading
empty element. The path "/" has two empty elements, which seems a bit
odd, but makes everything else work generally, especially the parser.
A single empty element doesn't make much sense, but is handled okay,
and treated as if it is a fully empty path.

Add a helper function for easily appending a path element without
creating a double-slash (replaces the last element with new element
if the last element is empty).

Add some unit tests, and also fix a few minor URI bugs:
 * scheme was defined incorrectly (it must start with an alpha)
 * Fix serializing of a URI with no authority, and an absolute path
   starting with two slashes
 * Fix transform where the base uri does not have a path

Change-Id: Ib6b86a14886dabfe3f717631c56323b768ea44dc
Reviewed-on: https://gerrit.dechocorp.com/10207
Reviewed-by: Punky Brewster (labs admin) <punkyb@example.com>
Reviewed-by: Jeremy Stanley <jeremy@mozy.com>
  • Loading branch information
@ccutrer
ccutrer committed Aug 30, 2010
1 parent 10ddc74 commit b72b1dc
Show file tree
Hide file tree
Showing 8 changed files with 278 additions and 107 deletions.
30 changes: 12 additions & 18 deletions autoexp.dat
View file
Expand Up @@ -10,7 +10,7 @@ Mordor::URI{
";")
) #else ( "" ),
#if ($e.authority.m_hostDefined) ( #("//", $e.authority) ) #else ( "" ),
#if ($e.path.type == ABSOLUTE || $e.path.segments._Myfirst != $e.path.segments._Mylast) ( $e.path ) #else ( "" ),
#if ($e.path.segments._Myfirst != $e.path.segments._Mylast) ( $e.path ) #else ( "" ),
#if ($e.m_queryDefined) ( #(
"?",
#if (($e.m_query._Myres) < ($e.m_query._BUF_SIZE)) ( [$e.m_query._Bx._Buf,sb] ) #else ( [$e.m_query._Bx._Ptr,sb] ))
Expand All @@ -25,7 +25,7 @@ Mordor::URI{
#(
#if ($e.m_schemeDefined) ( #(scheme: $e.m_scheme) ) #else ( #array(expr: 0, size: 0) ),
#if ($e.authority.m_hostDefined) ( #(authority: $e.authority) ) #else ( #array(expr: 0, size: 0) ),
#if ($e.path.type == ABSOLUTE || $e.path.segments._Myfirst != $e.path.segments._Mylast) ( #(path: $e.path) ) #else ( #array(expr: 0, size: 0) ),
#if ($e.path.segments._Myfirst != $e.path.segments._Mylast) ( #(path: $e.path) ) #else ( #array(expr: 0, size: 0) ),
#if ($e.m_queryDefined) ( #(query: $e.m_query) ) #else ( #array(expr: 0, size: 0) ),
#if ($e.m_fragmentDefined) ( #(fragment: $e.m_fragment) ) #else ( #array(expr: 0, size: 0) ),
#(Actual Members: [$e,!])
Expand Down Expand Up @@ -53,24 +53,18 @@ Mordor::URI::Authority{

Mordor::URI::Path{
preview (
#if ($e.type == RELATIVE && $e.segments._Myfirst == $e.segments._Mylast) (
#if ($e.segments._Myfirst == $e.segments._Mylast) (
""
) #else (
#(
#if ($e.type == ABSOLUTE) (
"/"
) #else (
""
),
#array(
expr:
#if ((($e.segments._Myfirst[$i])._Myres) < (($e.segments._Myfirst[$i])._BUF_SIZE)) (
[($e.segments._Myfirst[$i])._Bx._Buf,sb]
) #else (
[($e.segments._Myfirst[$i])._Bx._Ptr,sb]
),
size: $e.segments._Mylast - $e.segments._Myfirst
)
#array(
expr:
;[$e.segments._Myfirst[$i],
#if ((($e.segments._Myfirst[$i])._Myres) < (($e.segments._Myfirst[$i])._BUF_SIZE)) (
[($e.segments._Myfirst[$i])._Bx._Buf,sb]
) #else (
[($e.segments._Myfirst[$i])._Bx._Ptr,sb]
),
size: $e.segments._Mylast - $e.segments._Myfirst
)
)
)
Expand Down
8 changes: 4 additions & 4 deletions mordor/http/http_parser.rl
View file
Expand Up @@ -475,7 +475,7 @@ unquote(const std::string &str)

action set_request_uri {
m_uri = &m_request->requestLine.uri;
m_path = &m_uri->path;
m_segments = &m_uri->path.segments;
}

action save_accept_list_element {
Expand Down Expand Up @@ -605,7 +605,7 @@ unquote(const std::string &str)

action set_referer {
m_uri = &m_request->request.referer;
m_path = &m_uri->path;
m_segments = &m_uri->path.segments;
}

action save_accept_attribute {
Expand Down Expand Up @@ -721,7 +721,7 @@ Parser::adjustPointers(ptrdiff_t offset)
RequestParser::RequestParser(Request& request)
: m_request(&request),
m_ver(&request.requestLine.ver),
m_path(&request.requestLine.uri.path),
m_segments(&request.requestLine.uri.path.segments),
m_general(&request.general),
m_entity(&request.entity)
{}
Expand Down Expand Up @@ -825,7 +825,7 @@ ResponseParser::ResponseParser(Response& response)
: m_response(&response),
m_ver(&response.status.ver),
m_uri(&response.response.location),
m_path(&response.response.location.path),
m_segments(&response.response.location.path.segments),
m_general(&response.general),
m_entity(&response.entity)
{}
Expand Down
4 changes: 2 additions & 2 deletions mordor/http/parser.h
View file
Expand Up @@ -55,7 +55,7 @@ class RequestParser : public Parser
Request *m_request;
Version *m_ver;
URI *m_uri;
URI::Path *m_path;
std::vector<std::string> *m_segments;
GeneralHeaders *m_general;
EntityHeaders *m_entity;
};
Expand All @@ -74,7 +74,7 @@ class ResponseParser : public Parser
Response *m_response;
Version *m_ver;
URI *m_uri;
URI::Path *m_path;
std::vector<std::string> *m_segments;
GeneralHeaders *m_general;
EntityHeaders *m_entity;
};
Expand Down
3 changes: 1 addition & 2 deletions mordor/http/proxy.cpp
View file
Expand Up @@ -109,7 +109,6 @@ std::vector<URI> proxyFromList(const URI &uri, const std::string &proxy,
proxyUri.scheme(uri.scheme());
}
proxyUri.path.segments.clear();
proxyUri.path.type = URI::Path::RELATIVE;
proxyUri.authority.userinfoDefined(false);
proxyUri.queryDefined(false);
proxyUri.fragmentDefined(false);
Expand Down Expand Up @@ -427,7 +426,7 @@ tunnel(RequestBroker::ptr requestBroker, const URI &proxy, const URI &target)
URI &requestUri = requestHeaders.requestLine.uri;
requestUri.scheme("http");
requestUri.authority = proxy.authority;
requestUri.path.type = URI::Path::ABSOLUTE;
requestUri.path.segments.push_back(std::string());
requestUri.path.segments.push_back(os.str());
requestHeaders.request.host = os.str();
requestHeaders.general.connection.insert("Proxy-Connection");
Expand Down
8 changes: 3 additions & 5 deletions mordor/tests/http_server.cpp
View file
Expand Up @@ -357,13 +357,11 @@ static void doRespondStream(RequestBroker &requestBroker, bool head,
Request requestHeaders;
if (head)
requestHeaders.requestLine.method = HEAD;
requestHeaders.requestLine.uri.scheme("http");
requestHeaders.requestLine.uri.authority.host("localhost");
requestHeaders.requestLine.uri.path.type = URI::Path::ABSOLUTE;
requestHeaders.requestLine.uri = "http://localhost/";
if (!seekable)
requestHeaders.requestLine.uri.path.segments.push_back("unseekable");
requestHeaders.requestLine.uri.path.append("unseekable");
if (!sizeable)
requestHeaders.requestLine.uri.path.segments.push_back("unsizeable");
requestHeaders.requestLine.uri.path.append("unsizeable");

// Request the whole stream
MemoryStream response;
Expand Down
108 changes: 105 additions & 3 deletions mordor/tests/uri.cpp
View file
Expand Up @@ -93,6 +93,8 @@ MORDOR_UNITTEST(URI, serializationAndParsing)
serializeAndParse("g#s/../x");
serializeAndParse("http://a/b/c/g#s/../x");
serializeAndParse("http:g");
serializeAndParse("http:/hi");
serializeAndParse("http:////hi");
}

MORDOR_UNITTEST(URI, pathNormalization)
Expand Down Expand Up @@ -123,7 +125,6 @@ MORDOR_UNITTEST(URI, normalization)
MORDOR_TEST_ASSERT(!lhs.authority.userinfoDefined());
MORDOR_TEST_ASSERT(!rhs.authority.userinfoDefined());
MORDOR_TEST_ASSERT_EQUAL(lhs.authority, rhs.authority);
MORDOR_TEST_ASSERT_EQUAL(lhs.path.type, rhs.path.type);
MORDOR_TEST_ASSERT_EQUAL(lhs.path.segments, rhs.path.segments);
MORDOR_TEST_ASSERT_EQUAL(lhs.path, rhs.path);
MORDOR_TEST_ASSERT(!lhs.queryDefined());
Expand Down Expand Up @@ -183,6 +184,9 @@ MORDOR_UNITTEST(URI, transform)
MORDOR_TEST_ASSERT_EQUAL(URI::transform(base, URI("g#s/../x")), URI("http://a/b/c/g#s/../x"));

MORDOR_TEST_ASSERT_EQUAL(URI::transform(base, URI("http:g")), URI("http:g"));

MORDOR_TEST_ASSERT_EQUAL(URI::transform("http:hi", "bob"), "http:bob");
MORDOR_TEST_ASSERT_EQUAL(URI::transform("http://authority", "bob"), "http://authority/bob");
}

MORDOR_UNITTEST(URI, serializeCompleteOnBlockBoundary)
Expand Down Expand Up @@ -297,12 +301,12 @@ MORDOR_UNITTEST(URI, queryString)
MORDOR_UNITTEST(URI, encoding)
{
URI uri;
uri.path.type = URI::Path::ABSOLUTE;
uri.path.segments.push_back(std::string());
uri.path.segments.push_back("WiX Tutorial \xe2\x80\x94 Introduction to the Windows Installer XML Toolset.URL");
MORDOR_TEST_ASSERT_EQUAL(uri.toString(),
"/WiX%20Tutorial%20%E2%80%94%20Introduction%20to%20the%20Windows%20Installer%20XML%20Toolset.URL");

uri.path.segments[0] = "\xe5\xa4\x9a\xe8\xa8\x80\xe8\xaa\x9e\xe5\xaf\xbe\xe5\xbf\x9c\xe3\x82"
uri.path.segments[1] = "\xe5\xa4\x9a\xe8\xa8\x80\xe8\xaa\x9e\xe5\xaf\xbe\xe5\xbf\x9c\xe3\x82"
"\xb5\xe3\x83\xbc\xe3\x83\x81\xe3\x82\xa8\xe3\x83\xb3\xe3\x82\xb8\xe3"
"\x83\xb3\xe3\x81\xae\xe6\x97\xa5\xe6\x9c\xac\xe7\x89\x88\xe3\x80\x82"
"\xe3\x82\xa6\xe3\x82\xa7\xe3\x83\x96\xe3\x80\x81\xe3\x82\xa4\xe3\x83"
Expand All @@ -316,3 +320,101 @@ MORDOR_UNITTEST(URI, encoding)
"%A1%E3%83%BC%E3%82%B8%E3%81%8A%E3%82%88%E3%81%B3%E3"
"%83%8B%E3%83%A5%E3%83%BC%E3%82%B9%E6%A4%9C%E7%B4%A2.txt");
}

MORDOR_UNITTEST(URI, emptyFirstComponent)
{
serializeAndParse("/");
serializeAndParse("http://localhost/");
serializeAndParse("http://localhost//");
serializeAndParse("http://localhost///");

URI::Path path;
path = "/";
MORDOR_TEST_ASSERT_EQUAL(path.segments.size(), 2u);
MORDOR_TEST_ASSERT(path.isAbsolute());
MORDOR_TEST_ASSERT(path.segments.front().empty());
MORDOR_TEST_ASSERT(path.segments.back().empty());
path = "";
MORDOR_TEST_ASSERT(path.isRelative());
MORDOR_TEST_ASSERT(path.segments.empty());
path = "//";
MORDOR_TEST_ASSERT(path.isAbsolute());
MORDOR_TEST_ASSERT_EQUAL(path.segments.size(), 3u);
path = "a//";
MORDOR_TEST_ASSERT(path.isRelative());
MORDOR_TEST_ASSERT_EQUAL(path.segments.size(), 3u);

URI uri = "http://localhost/";
MORDOR_TEST_ASSERT_EQUAL(uri.toString(), "http://localhost/");
uri.path.append("hi");
MORDOR_TEST_ASSERT_EQUAL(uri.toString(), "http://localhost/hi");
uri.path.segments.push_back("");
MORDOR_TEST_ASSERT_EQUAL(uri.toString(), "http://localhost/hi/");
uri.path.segments.push_back("bye");
MORDOR_TEST_ASSERT_EQUAL(uri.toString(), "http://localhost/hi//bye");
uri.path.segments.push_back("");
MORDOR_TEST_ASSERT_EQUAL(uri.toString(), "http://localhost/hi//bye/");
uri.path.append("adios");
MORDOR_TEST_ASSERT_EQUAL(uri.toString(), "http://localhost/hi//bye/adios");
uri = "http://localhost/";
uri.path.append("hi");
MORDOR_TEST_ASSERT_EQUAL(uri.toString(), "http://localhost/hi");

// scheme == http, authority is not defined, path = "//hi";
// serialization has to add an extra // so it's not ambiguous with
// authority
uri = URI();
uri.scheme("http");
uri.path.segments.push_back(std::string());
uri.path.segments.push_back(std::string());
uri.path.segments.push_back("hi");
MORDOR_TEST_ASSERT_EQUAL(uri.toString(), "http:////hi");
}

MORDOR_UNITTEST(URI, append)
{
URI uri = "http://localhost";
uri.path.append("hi");
MORDOR_TEST_ASSERT_EQUAL(uri, "http://localhost/hi");
uri.path.append("bye");
MORDOR_TEST_ASSERT_EQUAL(uri, "http://localhost/hi/bye");
uri.path.append(std::string());
MORDOR_TEST_ASSERT_EQUAL(uri, "http://localhost/hi/bye/");
uri.path.append(std::string());
MORDOR_TEST_ASSERT_EQUAL(uri, "http://localhost/hi/bye/");

uri = "http:";
uri.path.append("hi");
MORDOR_TEST_ASSERT_EQUAL(uri, "http:hi");
uri.path.append("bye");
MORDOR_TEST_ASSERT_EQUAL(uri, "http:hi/bye");

URI::Path path;
path.append("hi");
MORDOR_TEST_ASSERT_EQUAL(path, "hi");
path.append("bye");
MORDOR_TEST_ASSERT_EQUAL(path, "hi/bye");
}

MORDOR_UNITTEST(URI, makeAbsolute)
{
URI::Path path;
path.makeAbsolute();
MORDOR_TEST_ASSERT_EQUAL(path, "/");
path.makeAbsolute();
MORDOR_TEST_ASSERT_EQUAL(path, "/");
path.makeRelative();
MORDOR_TEST_ASSERT(path.segments.empty());
path.makeRelative();
MORDOR_TEST_ASSERT(path.segments.empty());
path.append("hi");
MORDOR_TEST_ASSERT_EQUAL(path, "hi");
path.makeAbsolute();
MORDOR_TEST_ASSERT_EQUAL(path, "/hi");
path.makeAbsolute();
MORDOR_TEST_ASSERT_EQUAL(path, "/hi");
path.makeRelative();
MORDOR_TEST_ASSERT_EQUAL(path, "hi");
path.makeRelative();
MORDOR_TEST_ASSERT_EQUAL(path, "hi");
}
53 changes: 42 additions & 11 deletions mordor/uri.h
View file
Expand Up @@ -90,29 +90,57 @@ struct URI
};
Authority authority;

/// Represents segments in the path
///
/// A single, empty segment is invalid. A leading empty segment indicates
/// an absolute path; a trailing empty segment indicates a trailing slash.
struct Path
{
enum Type {
ABSOLUTE,
RELATIVE
};

friend struct URI;
private:
Path(const URI &uri);
public:
Path();
Path(const char *path);
Path(const std::string& path);

Path& operator=(const std::string& path);
Path& operator=(const char *path) { return *this = std::string(path); }

Type type;

bool isEmpty() const { return type == RELATIVE && segments.empty(); }

bool isEmpty() const
{
return segments.empty() ||
(segments.size() == 1 && segments.front().empty());
}
bool isAbsolute() const
{
return segments.size() > 1 && segments.front().empty();
}
bool isRelative() const
{
return !isAbsolute();
}

void makeAbsolute();
void makeRelative();

/// Append a single segment
///
/// This will remove a trailing empty segment before appending, if
/// necessary
/// I.e., Path("/hi/").append("bob") would result in "/hi/bob" instead
/// of "/hi//bob"
/// Also, if this path is part of a URI, and the URI has an authority
/// defined, and the path is empty, append will ensure the path becomes
/// absolute.
/// I.e., URI("http://localhost").path.append("bob") would result in
/// "http://localhost/bob"
void append(const std::string &segment);
void removeDotComponents();
void normalize(bool emptyPathValid = false);

// Concatenate rhs to this object, dropping least significant component
// of this object first
/// Concatenate rhs to this object, dropping least significant
/// component of this object first
void merge(const Path& rhs);

std::vector<std::string> segments;
Expand All @@ -130,6 +158,9 @@ struct URI
bool operator==(const Path &rhs) const;
bool operator!=(const Path &rhs) const
{ return !(*this == rhs); }

private:
const URI *m_uri;
};
Path path;

Expand Down

0 comments on commit b72b1dc

Please sign in to comment.