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

Tagging with invalid fromatted tag should be forbidden - return 400 #242

Open
1 of 6 tasks
Fak3 opened this issue Feb 11, 2017 · 2 comments
Open
1 of 6 tasks

Tagging with invalid fromatted tag should be forbidden - return 400 #242

Fak3 opened this issue Feb 11, 2017 · 2 comments
Labels
Milestone

Comments

@Fak3
Copy link
Contributor

Fak3 commented Feb 11, 2017

As we will have to put tags into url at some point, I think we should impose some restrictions on the format of a valid tag.

This is up to be discussed. I think we should only allow a-zA-Z0-9._- and no spaces.

Acceptance criteria

  • only a-zA-Z0-9._- and no spaces allowed.
  • user get's 400 Bad Request if tag is not allowed

Tasks

  • add failing tests
  • refactor tag_data_package(publisher, package) to pass the tests
  • change status code for ATTRIBUTE_MISSING to 404 (see analysis)
  • update tests

Analysis

In the function responsible for tagging we have code snippet

if 'version' not in data:
            return handle_error('ATTRIBUTE_MISSING', 'version not found', 400)

Meaning we are already returning 400 for other error.

Think we should use 400 for invalid Tagging as it usually used to indicate Bad Request. see https://en.wikipedia.org/wiki/List_of_HTTP_status_codes#4xx_Client_errors
So for ATTRIBUTE_MISSING we can use 404

@rufuspollock rufuspollock added this to the Backlog milestone Feb 14, 2017
@zelima
Copy link
Contributor

zelima commented Apr 26, 2017

Updated with acceptance criteria, tasks and analysis

@rufuspollock
Copy link
Contributor

@zelima good work and much clearer.

One item: i don't understand the if 'version' not in data snippet - what is it doing and why would we change to 404? BTW we don't need different error codes for different errors - we just need different error messages.

Finally, could you estimate this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants