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.

Sign up for GitHub

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

Jump to bottom

Modules: fix writing to read-only memory. #850

Merged
merged 1 commit into from
Feb 11, 2025
Merged

Modules: fix writing to read-only memory. #850

merged 1 commit into from
Feb 11, 2025

Conversation

xeioex
Copy link
Contributor

@xeioex xeioex commented Feb 8, 2025

No description provided.

@xeioex xeioex requested a review from hongzhidao February 8, 2025 06:46
@xeioex xeioex marked this pull request as ready for review February 8, 2025 06:57
hongzhidao
hongzhidao previously approved these changes Feb 10, 2025
View reviewed changes
Copy link
Contributor

@hongzhidao hongzhidao left a comment

Choose a reason for hiding this comment

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

The fix looks good.
It looks like doing something like preserver {some var related stuff} case sensitivity.
I mean we could make the commit title clearer.

@xeioex
Modules: fixed name corruption in variable and header processing.
802ee26 
The HTTP and Stream JS modules were performing in-place lowercasing of
variable and header names, which could inadvertently overwrite the
original data.

In the NJS engine, the problem did not manifest itself for strings up to
14 bytes long because they are inlined into the value.
@xeioex xeioex requested a review from hongzhidao February 10, 2025 23:36
@xeioex
Copy link
Contributor Author

xeioex commented Feb 10, 2025

@hongzhidao

  1. added extended commit log
  2. removed one comparison from the fast code pass
  3. added forgotten stack storage for ngx_stream_qjs_variables_set_property()

hongzhidao
hongzhidao approved these changes Feb 11, 2025
View reviewed changes
Copy link
Contributor

@hongzhidao hongzhidao left a comment

Choose a reason for hiding this comment

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

LGTM.

@xeioex xeioex merged commit ae7d4f4 into nginx:master Feb 11, 2025
1 check passed
@xeioex xeioex deleted the fix_readonly branch February 11, 2025 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hongzhidao hongzhidao hongzhidao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@xeioex @hongzhidao