Skip to content

fix: add set_url method to http_request for atomic URL updates#25

Merged
ruslanti merged 1 commit into
mainfrom
fix/set_url
May 22, 2026
Merged

fix: add set_url method to http_request for atomic URL updates#25
ruslanti merged 1 commit into
mainfrom
fix/set_url

Conversation

@ruslanti
Copy link
Copy Markdown
Collaborator

No description provided.

@ruslanti ruslanti self-assigned this May 22, 2026
Copilot AI review requested due to automatic review settings May 22, 2026 12:58
@ruslanti ruslanti requested a review from qrdl May 22, 2026 12:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the HttpRequest API to support consistent (“atomic”) request URL rewrites by introducing a single method that updates nginx’s uri, args, and unparsed_uri fields together.

Changes:

  • Add HttpRequest::set_url(path, query) to update uri, args, and unparsed_uri in one call and clear valid_unparsed_uri.
  • Add HttpRequest::uri() accessor.
  • Minor formatting fix in send_local_response output filter call.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +264 to +272
/// Atomically update the request URL: path, query string and unparsed_uri.
/// All three nginx fields (`uri`, `args`, `unparsed_uri`) are updated and
/// `valid_unparsed_uri` flag is cleared so nginx will rebuild it if needed.
///
/// `path` should be just the path (e.g. `/foo/bar`).
/// `query` should be the query string without leading `?` (empty for no query).
pub fn set_url(&mut self, path: &[u8], query: &[u8]) -> anyhow::Result<()> {
let pool = self
.pool()
Comment thread nginx_module/src/http_request.rs
@ruslanti ruslanti merged commit 4f3930f into main May 22, 2026
8 checks passed
@ruslanti ruslanti deleted the fix/set_url branch May 22, 2026 13:33
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.

3 participants