-
Notifications
You must be signed in to change notification settings - Fork 585
A new function, normalize and a test case #5
Changes from all commits
0dcea38
7219ad7
0c90ce0
f2825ca
1cbdf2d
13b82ed
fb8d986
692f12f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
// Copyright Fredrik Olofsson 2012. | ||
// Distributed under the Boost Software License, Version 1.0. | ||
// (See accompanying file LICENSE_1_0.txt or copy at | ||
// http://www.boost.org/LICENSE_1_0.txt) | ||
#ifndef __CPPNETLIB_NETWORK_URI_NORMALIZE__ | ||
#define __CPPNETLIB_NETWORK_URI_NORMALIZE__ | ||
|
||
#pragma once | ||
|
||
#include <string> | ||
|
||
namespace boost | ||
{ | ||
namespace network | ||
{ | ||
namespace uri | ||
{ | ||
/** | ||
* Normalize a given path and returns it. The path will be resolved and any attempt to | ||
* point to a file above (www) root will be removed. The returned string will allways | ||
* start with a slash ("/"). The path must be an instance of std::basic_string<>. The | ||
* function does handle wide characters as well, so both std::string and std::wstring | ||
* are accepted. Examples: | ||
* | ||
* "/test/test/../" : -> "/test" | ||
* "../../../" : -> "/" | ||
*/ | ||
template <typename Type> | ||
inline std::basic_string<Type> normalize(std::basic_string<Type> path) | ||
{ | ||
//why pass by value? See http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/ | ||
|
||
typedef std::basic_string<Type> StringType; | ||
|
||
//so the method can handle wide characters as well, otherwise must we use L"" or "" depending on char/wchar_t. | ||
static StringType slash1(1, '/'); | ||
static StringType slash2(2, '/'); | ||
static StringType dot2(2, '.'); | ||
static StringType dot2_slash1(dot2 + slash1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These should not be function local statics. If anything you should use the constants type where these values are already defined in a single static location. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You must do it when, as fast I just including network/constants.hpp I got some compile errors I cannot solve: 2>C:\cpp-netlib\branch\boost/network/constants.hpp(150): error C2065: 'unsupported_tag' : undeclared identifier The file is unmodified (constants.hpp) and is a week old. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See how it's used everywhere else then use it the same way here. |
||
|
||
if(path.empty() || path.front() != '/') | ||
path.insert(0, slash1); | ||
|
||
std::size_t //query_offset = path.find(question), | ||
temp = 0, | ||
pos = 0, | ||
counter = 0; | ||
|
||
while((pos = path.find(slash2)) != StringType::npos) // && pos < query_offset) | ||
path.replace(pos,2,slash1); | ||
|
||
|
||
while((pos = path.find(dot2_slash1)) != StringType::npos)// && pos < query_offset) | ||
{ | ||
temp = pos; | ||
while(0 < temp && counter < 2) | ||
{ | ||
if(path[--temp] == '/') | ||
++counter; | ||
} | ||
path.replace(temp, pos-temp+3,slash1); | ||
counter = 0; | ||
} | ||
|
||
pos = /*((query_offset == StringType::npos) ? */ path.size() /*: query_offset)*/ - 1; | ||
if(path[pos] == '/' && pos != 0) | ||
path.erase(pos,1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the point of removing the trailing '/'? This is not part of normalization of URIs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the simple reason why: "dir/sub/" is often the same path as "dir/sub". But I can remove it. |
||
|
||
return path; | ||
} | ||
|
||
//A simple helper method, deduce const wchar_t/char * to std::basic_string<char/wchar_t> | ||
template <typename Type> | ||
inline std::basic_string<Type> normalize(const Type* path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we supporting bare pointers here? I would rather this not be implemented and make normalize actually mutate an existing path. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I skip this part cannot visual studio deduce compile arguments: "could not deduce template argument for..." and will generate errors for a call like this: normalize("const char* here").The method will only simply convert a const char* or const wchar_t* to its corresponding string implementation, what is wrong with that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's wrong with that, is that you shouldn't really be supporting raw pointers. If someone wants to normalize a string or a wide string you do this:
|
||
{ | ||
return normalize(std::basic_string<Type>(path)); | ||
} | ||
} | ||
} | ||
} | ||
|
||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're not modifying the parameter, no need to pass by value. There is also no reason why this should be a "regular" function that takes a value and returns a new object. You might as well take the string as a non-cost lvalue reference and modify it in place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it will make calls like: normalize("something") impossible. Why not just create a wrapper function in that case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is exactly the point: you shouldn't allow
normalize("something")
when you obviously really want to modify a string using your normalization routine. What you should be supporting is:If anybody wants to use this function, they should do:
Also, this functionality sounds like it's already in Boost.Filesystem and is very specific to filesystem paths that I'm having a hard time seeing whether it's worth having in cpp-netlib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I see, in that case so should this pull request be closed, if you are not interested to have it. I just thought it could be a good thing to have in the library, especially when it remove any attempts to receive a directory above "wwwroot". But I came up with another thing, sending files in a effective way. See what I have written about it in "uri thread" on the Google groups thread. You can answer there as well, or maybe Glyn.
PS. I will let Glyn close this pull request, when you have decided if this function should be added or not, but if so, please let me know faster, so I don´t waste time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've replied to that thread. As far as this pull request is concerned I think it's better if you look at whether Boost.Filesystem does what you want to do before you abandon this work. If it doesn't then it might be worth adding this functionality to that library because this is about filesystem paths already and not URIs.
About not wasting time, the best way to avoid this is to let us know what you want to work on and communicate with us how you intend to do it or while you're doing it. Thanks for the effort and I'm looking forward to your other contributions in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have been busy, therefore late answer
Well, I currently only use the async_server and the uri class, so anything which make those classes better would I be interested in, specially in async_server implementation. Its robustness, exception safety, the possibility to add more "logging possibilities", performance and so on. Maybe a short web-server example would be a great thing to-do as well, because I already comfortable with async_server impl :-) But in that way, were should I write all questions? Github, google groups?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best place to let everyone know what you want to work on is the mailing list. There's no other place where the discussions happen but there.
Github issues are reserved for filing bugs. General support and development concerns are discussed in the mailing list (Google groups).
As for the web server example, look at https://github.com/cpp-netlib/cpp-netlib/blob/master/libs/network/example/http/fileserver.cpp which already does practically everything you'd want from a fileserver. If you want to extend that to do other things then be my guest and send a pull request against it.
Let's continue this discussion in the mailing list to get a larger audience involved.