Skip to content

Conversation

@alxndrsn
Copy link
Contributor

@alxndrsn alxndrsn commented Apr 28, 2025

Noted while working on #987

What has been done to verify that this works as intended?

Current tests continue to pass.

Why is this the best possible solution? Were any other approaches considered?

Using rewrite ... last and separating the two blocks means that passthrough URLs are now rewritten, and the entire config file re-evaluated for the new URLs:

stops processing the current set of ngx_http_rewrite_module directives and starts a search for a new location matching the changed URI;
- https://nginx.org/en/docs/http/ngx_http_rewrite_module.html#rewrite

This is in contrast to the current approach using rewrite ... break, which:

stops processing the current set of ngx_http_rewrite_module directives as with the break directive;
- https://nginx.org/en/docs/http/ngx_http_rewrite_module.html#rewrite

and the break directive is defined as:

If a directive is specified inside the location, further processing of the request continues in this location.

Which means that break, unintuitively, allows continuing of the current location block.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Before submitting this PR, please make sure you have:

  • branched off and targeted the next branch OR only changed documentation/infrastructure (master is stable and used in production)
  • verified that any code or assets from external sources are properly credited in comments or that everything is internally sourced

@alxndrsn alxndrsn requested a review from sadiqkhoja April 30, 2025 10:14
@alxndrsn alxndrsn marked this pull request as ready for review April 30, 2025 10:14
@alxndrsn alxndrsn merged commit 7a5ebb6 into getodk:next May 2, 2025
9 of 10 checks passed
@alxndrsn alxndrsn deleted the simplify-enketo-rules branch May 2, 2025 09:01
sadiqkhoja added a commit that referenced this pull request May 2, 2025
sadiqkhoja added a commit that referenced this pull request May 2, 2025
sadiqkhoja added a commit that referenced this pull request May 2, 2025
* Revert "nginx: simplify enketo passthrough rewrites (#992)"

This reverts commit 7a5ebb6.

* Add a test case that fails in #992
alxndrsn pushed a commit to alxndrsn/odk-central that referenced this pull request Jun 5, 2025
Re-introduce change from getodk#992 which was reverted in getodk#1018.
yanokwa pushed a commit to yanokwa/odk-central that referenced this pull request Jun 11, 2025
yanokwa pushed a commit to yanokwa/odk-central that referenced this pull request Jun 11, 2025
* Revert "nginx: simplify enketo passthrough rewrites (getodk#992)"

This reverts commit 7a5ebb6.

* Add a test case that fails in getodk#992
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