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

fix(server): enable regex crate unicode-perl feature #1714

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented Mar 6, 2024

Enable the unicode-perl feature flag on regex crate dependency in neqo-server crate.

The following regex makes use of the feature:

Regex::new(r"GET +/(\S+)(?:\r)?\n").unwrap()

Without the feature, the QUIC Interop tests panic with:

server  | thread 'main' panicked at neqo-server/src/old_https.rs:140:49:
server  | called `Result::unwrap()` on an `Err` value: Syntax(
server  | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
server  | regex parse error:
server  |     GET +/(\S+)(?:\r)?\n
server  |            ^^
server  | error: Unicode-aware Perl class not found (make sure the unicode-perl feature is enabled)
server  | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
server  | )

I will prioritize #1682 more to make sure we catch these regressions before merge.

Enable the `unicode-perl` feature flag on `regex` crate dependency in
`neqo-server` crate.

The following regex makes use of the feature:

``` rust
Regex::new(r"GET +/(\S+)(?:\r)?\n").unwrap()
```

Without the feature, the QUIC Interop tests panic with:

```
server  | thread 'main' panicked at neqo-server/src/old_https.rs:140:49:
server  | called `Result::unwrap()` on an `Err` value: Syntax(
server  | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
server  | regex parse error:
server  |     GET +/(\S+)(?:\r)?\n
server  |            ^^
server  | error: Unicode-aware Perl class not found (make sure the unicode-perl feature is enabled)
server  | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
server  | )
```
@larseggert larseggert added this pull request to the merge queue Mar 6, 2024
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.98%. Comparing base (a6bf979) to head (a28c68a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1714   +/-   ##
=======================================
  Coverage   92.98%   92.98%           
=======================================
  Files         120      120           
  Lines       37399    37399           
=======================================
  Hits        34775    34775           
  Misses       2624     2624           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Merged via the queue into mozilla:main with commit 6f407c9 Mar 6, 2024
13 checks passed
@mxinden
Copy link
Collaborator Author

mxinden commented Mar 6, 2024

Thanks @larseggert.

Can you trigger the QNS workflow on main so that https://interop.seemann.io/ eventually goes back to green?

https://github.com/mozilla/neqo/actions/workflows/qns.yml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants