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

Jump to bottom

Use empty nonce if not md5 #17487

Merged
merged 3 commits into from May 26, 2021
Merged

Use empty nonce if not md5 #17487

merged 3 commits into from May 26, 2021

Conversation

flamisz
Copy link
Contributor

@flamisz flamisz commented Apr 22, 2021 •
edited

Description:

If the nonce parameter is valid, ignore it.

Review

  • Functional review done
  • Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@flamisz flamisz self-assigned this Apr 22, 2021
@flamisz flamisz added Needs Review PRs that need a code review not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. labels Apr 22, 2021
@flamisz flamisz added this to the 4.3.0 milestone Apr 22, 2021
diosmosis
diosmosis reviewed Apr 23, 2021
View reviewed changes
core/Nonce.php Outdated Show resolved Hide resolved
tsteur
tsteur reviewed Apr 23, 2021
View reviewed changes
core/Nonce.php Outdated Show resolved Hide resolved
@flamisz
Copy link
Contributor Author

flamisz commented May 12, 2021

Is there anything else to do here @tsteur?

@tsteur
Copy link
Member

tsteur commented May 12, 2021

I haven't tested it but looks good to me @flamisz . We'd only merge it after the 4.3 release though just to make sure we're not breaking anything

diosmosis
diosmosis approved these changes May 13, 2021
View reviewed changes
Copy link
Member

@diosmosis diosmosis left a comment

Choose a reason for hiding this comment

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

Tested locally, works. Code looks good too.

@sgiehl sgiehl removed the Needs Review PRs that need a code review label May 17, 2021
@sgiehl sgiehl modified the milestones: 4.3.0, 4.4.0 May 18, 2021
@mattab mattab modified the milestones: 4.4.0, 4.3.0 May 26, 2021
@sgiehl sgiehl merged commit 27965ab into 4.x-dev May 26, 2021
@sgiehl sgiehl deleted the optoutnonceparameter branch May 26, 2021 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsteur tsteur tsteur left review comments

@diosmosis diosmosis diosmosis approved these changes

Assignees

@flamisz flamisz

Labels
not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Milestone
4.4.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@flamisz @tsteur @diosmosis @mattab @sgiehl