Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: query_rule consistent interpretation #865

Merged
merged 6 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,13 @@ jobs:
subrange-policy: |
msvc: one-per-minor
standards: '>=11'
latest-factors: ''
latest-factors: |
gcc ASan
clang Fuzz
factors: |
gcc Coverage ASan UBSan Shared No-Threads
gcc UBSan Coverage Shared No-Threads
msvc Shared x86
clang ASan UBSan IntegerSan Time-Trace Fuzz
clang ASan UBSan IntSan BoundsSan Time-Trace
mingw Shared
trace-commands: true

Expand Down Expand Up @@ -166,9 +168,9 @@ jobs:
echo 'CMAKE_GENERATOR_TOOLSET=${{ matrix.generator-toolset || '' }}' >> $GITHUB_ENV
echo 'CMAKE_BUILD_TYPE=${{ matrix.build-type || '' }}' >> $GITHUB_ENV
echo 'CC=${{ steps.setup-cpp.outputs.cc || matrix.cc || '' }}' >> $GITHUB_ENV
echo 'CFLAGS=${{ (matrix.integersan && '-fsanitize=integer -fno-sanitize=unsigned-integer-overflow -fno-sanitize-recover=integer -fno-omit-frame-pointer') || matrix.ccflags || '' }}' >> $GITHUB_ENV
echo 'CFLAGS=${{ (matrix.intsan && '-fsanitize=integer -fno-sanitize=unsigned-integer-overflow -fno-sanitize-recover=integer -fno-omit-frame-pointer') || (matrix.boundssan && '-fsanitize=bounds -fno-sanitize-recover=bounds -fno-omit-frame-pointer') || matrix.ccflags || '' }}' >> $GITHUB_ENV
echo 'CXX=${{ steps.setup-cpp.outputs.cxx || matrix.cxx || '' }}' >> $GITHUB_ENV
echo 'CXXFLAGS=${{ (matrix.integersan && '-fsanitize=integer -fno-sanitize=unsigned-integer-overflow -fno-sanitize-recover=integer -fno-omit-frame-pointer') || matrix.cxxflags || '' }}' >> $GITHUB_ENV
echo 'CXXFLAGS=${{ (matrix.intsan && '-fsanitize=integer -fno-sanitize=unsigned-integer-overflow -fno-sanitize-recover=integer -fno-omit-frame-pointer') || (matrix.boundssan && '-fsanitize=bounds -fno-sanitize-recover=bounds -fno-omit-frame-pointer') || matrix.cxxflags || '' }}' >> $GITHUB_ENV
echo 'CXXSTD=${{ matrix.latest-cxxstd || '' }}' >> $GITHUB_ENV
echo 'BUILD_SHARED_LIBS=${{ matrix.shared || '' }}' >> $GITHUB_ENV
echo 'CMAKE_RUN_TESTS=true' >> $GITHUB_ENV
Expand Down Expand Up @@ -261,11 +263,11 @@ jobs:
cxxstd: ${{ ( matrix.is-latest && ((matrix.cxxstd && format('{0},latest', matrix.cxxstd)) || 'latest' )) || matrix.cxxstd }}
address-model: ${{ (matrix.x86 && '32') || '64' }}
asan: ${{ matrix.asan }}
ubsan: ${{ matrix.ubsan || matrix.integersan }}
ubsan: ${{ matrix.ubsan || matrix.intsan }}
tsan: ${{ matrix.tsan }}
# integer-sanitizer is passed directly as flags
ccflags: ${{ (matrix.integersan && '-fsanitize=integer -fno-sanitize=unsigned-integer-overflow -fno-sanitize-recover=integer -fno-omit-frame-pointer') || '' }}
cxxflags: ${{ (matrix.integersan && '-fsanitize=integer -fno-sanitize=unsigned-integer-overflow -fno-sanitize-recover=integer -fno-omit-frame-pointer') || '' }}
ccflags: ${{ (matrix.intsan && '-fsanitize=integer -fno-sanitize=unsigned-integer-overflow -fno-sanitize-recover=integer -fno-omit-frame-pointer') || '' }}
cxxflags: ${{ (matrix.intsan && '-fsanitize=integer -fno-sanitize=unsigned-integer-overflow -fno-sanitize-recover=integer -fno-omit-frame-pointer') || '' }}
shared: ${{ matrix.shared }}
coverage: ${{ matrix.coverage }}
define: ${{ (matrix.no-threads && 'BOOST_URL_DISABLE_THREADS') || '' }}
Expand Down
2 changes: 0 additions & 2 deletions doc/local-playbook.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ asciidoc:
attributes:
# Enable pagination
page-pagination: ''
# Remove the sidenav and include TOC in index.adoc page
remove-sidenav: ''
extensions:
- '@alandefreitas/asciidoctor-boost-links'
- '@asciidoctor/tabs'
90 changes: 64 additions & 26 deletions doc/modules/ROOT/pages/urls/params.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,17 @@ This URL has two query parameters, cpp:first[] and cpp:last[] whose values are "
https://www.example.com?first=John&last=Doe
----

[cols="2,3"]
|===
// Headers
|s| `params( s )`

// Row 1, Column 1
|`"?first=John&last=Doe"`
// Row 1, Column 2
|`{ { "first", "John" }, { "last", "Doe" } }`
|===

Like the path, the library permits access to the params as using these separate, bidirectional view types which reference the underlying URL:

[cols="1,1,3"]
Expand Down Expand Up @@ -70,7 +81,34 @@ Like the path, the library permits access to the params as using these separate,

A param always has a key, even if it is the empty string.
The value is optional; an empty string is distinct from no value.
To represent individual params the library uses these types, distinguished by their ownership model and whether or not percent-escapes are possible:
The lack of an `=` character in the query param is interpreted as no value, while an `=` character with no value is interpreted as an empty string.

[cols="2,3"]
|===
// Headers
|s| `params( s )`

|`"?name=John"`
|`{ { "name", "John" } }`

|`"?=John"`
|`{ { "", "John" } }`

|`"?name="`
|`{ { "name", "" } }`

|`"?name"`
|`{ { "name", no_value } }`

|`"?"`
|`{ { "", no_value } }`

|`""`
|`{}`

|===

To represent individual params the library uses these types, distinguished by their ownership model and whether percent-escapes are possible:

[cols="1,1,3"]
|===
Expand Down Expand Up @@ -117,38 +155,38 @@ This table shows some examples of initializer lists used to construct a param ty
|Statement|cpp:qp.key[]|cpp:qp.value[]|cpp:qp.has_value[]

// Row 1, Column 1
|cpp:param qp = { "first", "John" };[]
|`param qp = { "first", "John" };`
// Row 1, Column 2
|cpp:"First"[]
|`"First"`
// Row 1, Column 3
|cpp:"John"[]
|`"John"`
// Row 1, Column 4
|cpp:true[]

// Row 2, Column 1
|cpp:param qp = { "first", "" };[]
|`param qp = { "first", "" };`
// Row 2, Column 2
|cpp:"First"[]
|`"First"`
// Row 2, Column 3
|cpp:""[]
|`""`
// Row 2, Column 4
|cpp:true[]

// Row 3, Column 1
|cpp:param qp = { "first", no_value };[]
|`param qp = { "first", no_value };`
// Row 3, Column 2
|cpp:"First"[]
|`"First"`
// Row 3, Column 3
|cpp:""[]
|`""`
// Row 3, Column 4
|cpp:false[]

// Row 4, Column 1
|cpp:param qp = { "", "Doe" };[]
|`param qp = { "", "Doe" };`
// Row 4, Column 2
|cpp:""[]
|`""`
// Row 4, Column 3
|cpp:"Doe"[]
|`"Doe"`
// Row 4, Column 4
|cpp:true[]

Expand All @@ -167,37 +205,37 @@ This demonstrates how the syntax of the query maps to the parameter structure:
[cols="2,3"]
|===
// Headers
|s|cpp:parms( s )[]
|s| `params( s )`

// Row 1, Column 1
|cpp:"?first=John&last=Doe"[]
|`"?first=John&last=Doe"`
// Row 1, Column 2
|cpp:{ { "first", "John" }, { "last", "Doe" } }[]
|`{ { "first", "John" }, { "last", "Doe" } }`

// Row 2, Column 1
|cpp:"?id=42&unsorted"[]
|`"?id=42&unsorted"`
// Row 2, Column 2
|cpp:{ { "id", "42" }, { "last", no_value } }[]
|`{ { "id", "42" }, { "last", no_value } }`

// Row 3, Column 1
|cpp:"?col=cust&row="[]
|`"?col=cust&row="`
// Row 3, Column 2
|cpp:{ { "col", "cust" }, { "row", "" } }[]
|`{ { "col", "cust" }, { "row", "" } }`

// Row 4, Column 1
|cpp:"?justify=left&"[]
|`"?justify=left&"`
// Row 4, Column 2
|cpp:{ { "justify", "left" }, { "", no_value } }[]
|`{ { "justify", "left" }, { "", no_value } }`

// Row 5, Column 1
|cpp:"?"[]
|`"?"`
// Row 5, Column 2
|cpp:{ { "", no_value } }[]
|`{ { "", no_value } }`

// Row 6, Column 1
|cpp:""[]
|`""`
// Row 6, Column 2
|cpp:{ }[]
|`{ }`

|===

Expand Down
6 changes: 3 additions & 3 deletions include/boost/url/grammar/impl/variant_rule.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ parse_variant(
std::integral_constant<
std::size_t, I> const&,
std::false_type const&) ->
system::result<variant<
system::result<variant2::variant<
typename R0::value_type,
typename Rn::value_type...>>
{
Expand All @@ -57,15 +57,15 @@ parse_variant(
std::integral_constant<
std::size_t, I> const&,
std::true_type const&) ->
system::result<variant<
system::result<variant2::variant<
typename R0::value_type,
typename Rn::value_type...>>
{
auto const it0 = it;
auto rv = parse(
it, end, get<I>(rn));
if( rv )
return variant<
return variant2::variant<
typename R0::value_type,
typename Rn::value_type...>{
variant2::in_place_index_t<I>{}, *rv};
Expand Down
3 changes: 1 addition & 2 deletions src/rfc/absolute_uri_rule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,7 @@ parse(
// map "?" to { {} }
u.apply_query(
rv->query,
rv->count +
rv->query.empty());
rv->count);
}
}

Expand Down
7 changes: 5 additions & 2 deletions src/rfc/detail/query_part_rule.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,14 @@ struct query_part_rule_t
++it;
auto rv = grammar::parse(
it, end, query_rule);
if(! rv)
return rv.error();
// query_rule is optionally empty and must not fail
BOOST_ASSERT( rv );
value_type t;
t.query = rv->buffer();
t.count = rv->size();
// a query_rule represents at least one parameter:
// <empty key, no value>
BOOST_ASSERT( t.count );
t.has_query = true;
return t;
}
Expand Down
3 changes: 1 addition & 2 deletions src/rfc/origin_form_rule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ parse(
// map "?" to { {} }
u.apply_query(
rv->query,
rv->count +
rv->query.empty());
rv->count);
}
}

Expand Down
27 changes: 10 additions & 17 deletions src/rfc/query_rule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ parse(
{
if(it == end)
{
// empty string = 0 params
// empty string = 1 param
core::string_view str(it, 0);
return params_encoded_view(
detail::query_ref(
core::string_view(it, 0), 0, 0));
detail::query_ref(str, 0, 1));
}
auto const it0 = it;
std::size_t dn = 0;
Expand All @@ -50,18 +50,12 @@ parse(
}
if(*it == '%')
{
if(end - it < 3)
if(end - it < 3 ||
(!grammar::hexdig_chars(it[1]) ||
!grammar::hexdig_chars(it[2])))
{
// missing HEXDIG
BOOST_URL_RETURN_EC(
error::missing_pct_hexdig);
}
if (!grammar::hexdig_chars(it[1]) ||
!grammar::hexdig_chars(it[2]))
{
// expected HEXDIG
BOOST_URL_RETURN_EC(
error::bad_pct_hexdig);
// missing valid HEXDIG
break;
}
it += 3;
dn += 2;
Expand All @@ -71,11 +65,10 @@ parse(
break;
}
std::size_t const n(it - it0);
core::string_view str(it0, n);
return params_encoded_view(
detail::query_ref(
core::string_view(it0, n),
n - dn,
nparam));
str, n - dn, nparam));
}

} // urls
Expand Down
8 changes: 4 additions & 4 deletions src/rfc/relative_ref_rule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ parse(
it, end, detail::query_part_rule);
if(! rv)
return rv.error();
if(rv->has_query)
auto& v = *rv;
if(v.has_query)
{
// map "?" to { {} }
u.apply_query(
rv->query,
rv->count +
rv->query.empty());
v.query,
v.count);
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/rfc/uri_rule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ parse(
// map "?" to { {} }
u.apply_query(
rv->query,
rv->count +
rv->query.empty());
rv->count);
}
}

Expand Down
9 changes: 5 additions & 4 deletions test/cmake_test/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@
#include <boost/url.hpp>

int main() {
boost::urls::ipv4_address ip_addr({127, 0, 0, 1});
if (ip_addr.to_bytes().size() != 4) {
throw;
}
boost::urls::ipv4_address ip_addr({127, 0, 0, 1});
if (ip_addr.to_bytes().size() != 4) {
return 1;
}
return 0;
}
4 changes: 2 additions & 2 deletions test/unit/grammar/variant_rule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ struct variant_rule_test
(void)rv;
}

ok(r, "(", variant<core::string_view, core::string_view>(
ok(r, "(", variant2::variant<core::string_view, core::string_view>(
variant2::in_place_index_t<0>{}, "("));
ok(r, ")", variant<core::string_view, core::string_view>(
ok(r, ")", variant2::variant<core::string_view, core::string_view>(
variant2::in_place_index_t<1>{}, ")"));
bad(r, "", error::mismatch);
bad(r, "[]", error::mismatch);
Expand Down
Loading
Loading