Url decoding is only applied on the query parameters

[paulharris]

If I GET a url such as:
https://myserver.com/imgs/image with space.png?param=one two
the URL seen by the server is:
https://myserver.com/imgs/image%20with%20space.png?param=one%20two

The query parameters are decoded correctly, into:
param --> one two

However, the URL is still imgs/image%20with%20space.png

Shouldn't crowcpp automatically decode the URL path before it is passed to a route handler?

ie I would expect my handler to see imgs/image with space.png
Or are we expected to manually decode this?

[gittiver]

Crow currently does not decode the URL path.
Such a decoder could be added, but only makes sense in a few cases and adds additional runtime to request handling.
Therefore there is no plan to do the URL path decoding in crow.

[paulharris]

Ok, I'm fine with this, however, is this typical for webservers? It seems like surprising behaviour.

Can we document this?
And add a nice obviously named function?

My route was coded something like:

CROW_ROUTE(app, "/<string>/<path>")
([]( const crow::request& req, crow::response& res, const string_view url_leaf, string url_path_partial )
{
   // decode URL path
   url_path_partial.resize( crow::qs_decode(&url_path_partial[0]) );
}

IF this is the correct approach, then it would be nice to have a function that accepts a string and in-place decodes it.

What do you think?

[gittiver]

Yo are welcome to add the function and the documentation.

[paulharris]

I've pushed a PR with test case. Was trying codespaces, is there a way to run the unit tests in there?

[gittiver]

https://github.com/CrowCpp/Crow/pull/993/checks
they run now, I had to approve.

[paulharris]

I've forced-pushed. Do you have to re-approve each time?

[gittiver]

It seems. :)

[matelich]

Ok, I'm fine with this, however, is this typical for webservers? It seems like surprising behaviour.

Certainly surprised me, spent a long time assuming I was doing something wrong.
I'll be adding url_decode to my add_static_dir(), thanks!

Looking at the MSVC implementation of std::string::resize made me wrinkle my nose a bit for a non-changing case, so I'm using

      int decoded_length = url_decode(&url[0]);
      if (decoded_length != static_cast<int>(url.size()))
         url.resize(decoded_length);

[paulharris]

I'm curious so I looked into this ...

    _CONSTEXPR20 void resize(_CRT_GUARDOVERFLOW const size_type _New_size, const _Elem _Ch = _Elem()) {
        // determine new length, padding with _Ch elements as needed
        const size_type _Old_size = size();
        if (_New_size <= _Old_size) {
            _Eos(_New_size);
        } else {
            append(_New_size - _Old_size, _Ch);
        }
    }

For non-changing it always calls _Eos()

    _CONSTEXPR20 void _Eos(const size_type _New_size) noexcept { // set new length and null terminator
        _ASAN_STRING_MODIFY(*this, _Mypair._Myval2._Mysize, _New_size);
        _Mypair._Myval2._Mysize = _New_size;
        _Traits::assign(_Mypair._Myval2._Myptr()[_New_size], _Elem());
    }

For the non-changing case,
So it always sets size=size and then assigns the null terminator.

I guess they are trying to avoid a second test to check for == (one extra branch costs more than 2 assigns?)
libc++ also implements resize the same way:
https://github.com/llvm-mirror/libcxx/blob/78d6a7767ed57b50122a161b91f59f19c9bd0d19/include/string#L3095

libstdc++ uses a second test:
https://github.com/gcc-mirror/gcc/blob/24a041ea863721f3181e4433195e7697bf52c413/libstdc%2B%2B-v3/include/bits/basic_string.tcc#L432