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

Parse Integer negative value validation #16920

Merged
merged 14 commits into from
Apr 16, 2024
Merged

Parse Integer negative value validation #16920

merged 14 commits into from
Apr 16, 2024

Conversation

TrevisGordan
Copy link
Contributor

@TrevisGordan TrevisGordan commented Feb 14, 2024 •
edited
Loading

Overview

  • Expands function parse_integer with an optional negative value check and
    expands function parse_integer_from_args to raise INVALID_PARAM on a forbidden negative value.

  • Adds limit parameter negative value validation check
    to fix publicRooms validation error & parse_integer_from_args changes #16918 (500 internal server error on negative limit parameter)

  • Adds negative value validation to parse_integer functions.
    Removing duplicate negative value check logics blocks.

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct
    (run the linters)

@TrevisGordan
expands function parse_integer with an optional negative value check
daa1448 
expands function parse_integer_from_args to raise INVALID_PARAM on a forbidden negative value.
@TrevisGordan
Adds limit parameter negative value validation check
c63ba0a 
fixes element-hq#16918 500 internal server error on negative limit parameter (with PostgreSQL)
@TrevisGordan
Adds negative value validation to parse_integer functions.
2d18bef 
Removing duplicate negative value check logics blocks.
@TrevisGordan
adjusts hard-coded INVALID_PARAM error messages in media test cases.
00cf8ee
@TrevisGordan TrevisGordan requested a review from a team as a code owner February 14, 2024 14:24
reivilibre
reivilibre approved these changes Mar 19, 2024
View reviewed changes
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

Thanks!

@reivilibre reivilibre enabled auto-merge (squash) March 19, 2024 17:09
dklimpel

This comment was marked as outdated.

Sign in to view

auto-merge was automatically disabled March 20, 2024 09:58

Head branch was pushed to by a user without write access

dklimpel
dklimpel reviewed Mar 25, 2024
View reviewed changes
synapse/http/servlet.py Outdated Show resolved Hide resolved
reivilibre
reivilibre approved these changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this up!

@reivilibre reivilibre enabled auto-merge (squash) April 9, 2024 10:24
@TrevisGordan
Copy link
Contributor Author

Hey @reivilibre - I believe the prior systest fails were caused by outdated upstream mismatch.
After re-syncing / merging changes, the systests do pass 😳.
Could you just restart the test again - please (...before this is outdated again 🙈 Thank you! )

@reivilibre reivilibre merged commit f0d6f14 into element-hq:develop Apr 16, 2024
38 checks passed
yingziwu added a commit to yingziwu/synapse that referenced this pull request May 3, 2024
@yingziwu
Merge tag 'v1.106.0' into develop
168c76e 
No significant changes since 1.106.0rc1.

- Send an email if the address is already bound to an user account. ([\#16819](element-hq/synapse#16819))
- Implement the rendezvous mechanism described by [MSC4108](matrix-org/matrix-spec-proposals#4108). ([\#17056](element-hq/synapse#17056))
- Support delegating the rendezvous mechanism described [MSC4108](matrix-org/matrix-spec-proposals#4108) to an external implementation. ([\#17086](element-hq/synapse#17086))

- Add validation to ensure that the `limit` parameter on `/publicRooms` is non-negative. ([\#16920](element-hq/synapse#16920))
- Return `400 M_NOT_JSON` upon receiving invalid JSON in query parameters across various client and admin endpoints, rather than an internal server error. ([\#16923](element-hq/synapse#16923))
- Make the CSAPI endpoint `/keys/device_signing/upload` idempotent. ([\#16943](element-hq/synapse#16943))
- Redact membership events if the user requested erasure upon deactivating. ([\#17076](element-hq/synapse#17076))

- Add a prompt in the contributing guide to manually configure icu4c. ([\#17069](element-hq/synapse#17069))
- Clarify what part of message retention is still experimental. ([\#17099](element-hq/synapse#17099))

- Use new receipts column to optimise receipt and push action SQL queries. Contributed by Nick @ Beeper (@Fizzadar). ([\#17032](element-hq/synapse#17032), [\#17096](element-hq/synapse#17096))
- Fix mypy with latest Twisted release. ([\#17036](element-hq/synapse#17036))
- Bump minimum supported Rust version to 1.66.0. ([\#17079](element-hq/synapse#17079))
- Add helpers to transform Twisted requests to Rust http Requests/Responses. ([\#17081](element-hq/synapse#17081))
- Fix type annotation for `visited_chains` after `mypy` upgrade. ([\#17125](element-hq/synapse#17125))

* Bump anyhow from 1.0.81 to 1.0.82. ([\#17095](element-hq/synapse#17095))
* Bump peaceiris/actions-gh-pages from 3.9.3 to 4.0.0. ([\#17087](element-hq/synapse#17087))
* Bump peaceiris/actions-mdbook from 1.2.0 to 2.0.0. ([\#17089](element-hq/synapse#17089))
* Bump pyasn1-modules from 0.3.0 to 0.4.0. ([\#17093](element-hq/synapse#17093))
* Bump pygithub from 2.2.0 to 2.3.0. ([\#17092](element-hq/synapse#17092))
* Bump ruff from 0.3.5 to 0.3.7. ([\#17094](element-hq/synapse#17094))
* Bump sigstore/cosign-installer from 3.4.0 to 3.5.0. ([\#17088](element-hq/synapse#17088))
* Bump twine from 4.0.2 to 5.0.0. ([\#17091](element-hq/synapse#17091))
* Bump types-pillow from 10.2.0.20240406 to 10.2.0.20240415. ([\#17090](element-hq/synapse#17090))
@dkasak dkasak mentioned this pull request Jun 20, 2024
Merged
3 tasks
dkasak added a commit that referenced this pull request Jun 24, 2024
@dkasak
Tidy up integer parsing (#17339)
700d2cc 
The parse_integer function was previously made to reject negative values by
default in #16920, but the
documentation stated otherwise. This fixes the documentation and also:

- Removes explicit negative=False parameters from call sites.
- Brings the negative default of parse_integer_from_args in alignment with
  parse_integer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dklimpel dklimpel dklimpel left review comments

@reivilibre reivilibre reivilibre 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.

publicRooms validation error & parse_integer_from_args changes
3 participants
@TrevisGordan @dklimpel @reivilibre