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

Bugfix of regex at FloatConvertor #1942

Closed
wants to merge 1 commit into from
Closed

Bugfix of regex at FloatConvertor #1942

wants to merge 1 commit into from

Conversation

i-Ching
Copy link
Contributor

@i-Ching i-Ching commented Nov 13, 2022

  1. ValueError: could not convert string to float if path parameters '/path/{number:float}' like /path/1-1, /path/2+2, /path/3^3

  2. Bug: A dot in regex of class FloatConvetor is also a metacharacter, it is used to match any character.

  3. Fix: I need to add a backslash escape the dot, so regex = "[0-9]+(\.[0-9]+)?"

The starting point for contributions should usually be a discussion

Simple documentation typos may be raised as stand-alone pull requests, but otherwise please ensure you've discussed your proposal prior to issuing a pull request.

This will help us direct work appropriately, and ensure that any suggested changes have been okayed by the maintainers.

  • Initially raised as discussion #...

1. ValueError: could not convert string to float
if path parameters '/path/{number:float}' like /path/1-1, /path/2+2, /path/3^3

2. Bug: A dot in regex of class FloatConvetor is also a metacharacter, it is used to match any character.

3. Fix: I need to escape it, so regex = "[0-9]+(\.[0-9]+)?"
@Kludex
Copy link
Sponsor Member

Kludex commented Nov 13, 2022

Would you mind adding a test for it?

@i-Ching
Copy link
Contributor Author

i-Ching commented Nov 13, 2022

Would you mind adding a test for it?

Below is the simple test.py, I curl http://localhost:8000/path/1.1 is passed, but http://localhost:8000/path/2-2 raises 500 Server Error: ValueError: could not convert string to float: '2-2'

from starlette.applications import Starlette
from starlette.responses import JSONResponse
from starlette.routing import Route
import uvicorn

async def homepage(request):
    return JSONResponse(request.path_params)

app = Starlette(debug=True, routes=[
    Route('/path/{number:float}', homepage),
])
if __name__ == "__main__":
 uvicorn.run(app)

@i-Ching i-Ching closed this by deleting the head repository Nov 14, 2022
@i-Ching
Copy link
Contributor Author

i-Ching commented Nov 14, 2022

Sorry my bad, I deleted my fork to affect to close this pull request. It is related to the bugfix of #1944.
I reopen here and wait for your approval.

@i-Ching i-Ching reopened this Nov 14, 2022
@iudeen
Copy link

iudeen commented Nov 14, 2022

#1944 has the same goal, can we close this and continue there?

@i-Ching i-Ching closed this Nov 14, 2022
@Kludex Kludex mentioned this pull request Dec 3, 2022
Kludex added a commit that referenced this pull request Dec 3, 2022
* Bugfix of regex at FloatConvertor (version 2)

For passing your checks of #1942
A correct statement is: regex = r"[0-9]+(\.[0-9]+)?"
Reference: https://www.flake8rules.com/rules/W605.html

I have no problem to corrected without 'r' prefix directly at /site-packages/starlette/convertors.py of my computer.
Having submitted last pull-request, I realized to add a 'r' prefix to pass your tests.

* Add test

Co-authored-by: Ching <thus.kindly@gmail.com>
aminalaee pushed a commit that referenced this pull request Feb 13, 2023
* Bugfix of regex at FloatConvertor (version 2)

For passing your checks of #1942
A correct statement is: regex = r"[0-9]+(\.[0-9]+)?"
Reference: https://www.flake8rules.com/rules/W605.html

I have no problem to corrected without 'r' prefix directly at /site-packages/starlette/convertors.py of my computer.
Having submitted last pull-request, I realized to add a 'r' prefix to pass your tests.

* Add test

Co-authored-by: Ching <thus.kindly@gmail.com>
azurelotus0926 added a commit to azurelotus0926/starlette that referenced this pull request Jun 27, 2024
* Bugfix of regex at FloatConvertor (version 2)

For passing your checks of encode/starlette#1942
A correct statement is: regex = r"[0-9]+(\.[0-9]+)?"
Reference: https://www.flake8rules.com/rules/W605.html

I have no problem to corrected without 'r' prefix directly at /site-packages/starlette/convertors.py of my computer.
Having submitted last pull-request, I realized to add a 'r' prefix to pass your tests.

* Add test

Co-authored-by: Ching <thus.kindly@gmail.com>
github-actions bot pushed a commit to Kludex/jik that referenced this pull request Aug 16, 2024
* Bugfix of regex at FloatConvertor (version 2)

For passing your checks of encode/starlette#1942
A correct statement is: regex = r"[0-9]+(\.[0-9]+)?"
Reference: https://www.flake8rules.com/rules/W605.html

I have no problem to corrected without 'r' prefix directly at /site-packages/starlette/convertors.py of my computer.
Having submitted last pull-request, I realized to add a 'r' prefix to pass your tests.

* Add test

Co-authored-by: Ching <thus.kindly@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants