Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Fix user current update required email#4346

Merged
mitchell852 merged 4 commits into
apache:masterfrom
ocket8888:fix-user-current-update-required-email
Jan 29, 2020
Merged

Fix user current update required email#4346
mitchell852 merged 4 commits into
apache:masterfrom
ocket8888:fix-user-current-update-required-email

Conversation

@ocket8888

@ocket8888 ocket8888 commented Jan 28, 2020

Copy link
Copy Markdown
Contributor

What does this PR (Pull Request) do?

Adds a constraint that disallows setting the user's email address to an empty string in the PUT handler for /user/current.

Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Ops

What is the best way to verify this PR?

Run the client/api integration tests. I added a case that checks for empty string emails. Other than that, try submitting a PUT request to /user/current and verifying that the response indicates the error.

If this is a bug fix, what versions of Traffic Control are affected?

  • master

The following criteria are ALL met by this PR

  • This PR includes tests
  • This PR includes documentation
  • An update to CHANGELOG.md is not necessary
  • This PR includes any and all required license headers
  • This PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added bug something isn't working as intended Traffic Ops related to Traffic Ops documentation related to documentation low impact affects only a small portion of a CDN, and cannot itself break one labels Jan 28, 2020

@mhoppa mhoppa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it not handles empty strings correctly but if I send in null for the request for the email it causes a internal server error and a panic in TO

@ocket8888

Copy link
Copy Markdown
Contributor Author

I think that means it would have always done that, because I didn't move a dereference... weird that wasn't caught sooner.

@mhoppa mhoppa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

manual testing is successful, API tests pass

@mitchell852
mitchell852 merged commit fe1e92a into apache:master Jan 29, 2020
@ocket8888
ocket8888 deleted the fix-user-current-update-required-email branch January 29, 2020 16:54
@rawlinp

rawlinp commented Jan 30, 2020

Copy link
Copy Markdown
Contributor

This PR does not actually affect 4.0.x -- no backport necessary.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug something isn't working as intended documentation related to documentation low impact affects only a small portion of a CDN, and cannot itself break one Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can update a user's username to contain spaces

4 participants