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

app.run(port='5000', debug=True) error #2220

Closed
trigremm opened this issue Mar 23, 2017 · 7 comments · Fixed by pallets/werkzeug#1088
Closed

app.run(port='5000', debug=True) error #2220

trigremm opened this issue Mar 23, 2017 · 7 comments · Fixed by pallets/werkzeug#1088

Comments

@trigremm
Copy link

trigremm commented Mar 23, 2017

Inappropriate behaviour in app.run function with debug set to True and False

app.run(port='5000', debug=False) runs perfectly

however when debug=True

app.run(port='5000', debug=True) it throws error

Traceback (most recent call last):
  File "hello_world.py", line 13, in <module>
    app.run(port='5000', debug=True)
  File "/usr/lib64/python3.5/site-packages/flask/app.py", line 841, in run
    run_simple(host, port, self, **options)
  File "/usr/lib/python3.5/site-packages/werkzeug/serving.py", line 717, in run_simple
    s.bind((hostname, port))
TypeError: an integer is required (got type str)```
@ThiefMaster
Copy link
Member

Ports are numbers, not strings. However, I agree that it'd be nicer to fail in both cases.

@aaossa
Copy link

aaossa commented Mar 24, 2017

Can I submit a PR for this? I may have time this weekend. But, of course, if you want to do it you can go with it :)

Maybe a good solution is to check types and raise TypeError?

@untitaker
Copy link
Contributor

@aaossa Yes and yes. Also please add a test :)

@aaossa
Copy link

aaossa commented Mar 24, 2017

@untitaker, @ThiefMaster

Reading the source code, I thought, is this Werkzeug or Flask responsability? The error from the OP comment is raised from werkzeug.serving, because run_simple is from that module. We could check types after flask/app.py#L843 to avoid this problem without relying in Werkzeug. But we have another option: apply the same check at the top of run_simple (werkzeug/serving.py#L602) to (also) avoid the same error when using Werkzeug but not Flask.

Why does the same code work when using debug=False? Because debug mode uses the reloader (so use_reloader is set to True) and Werkzeug tries to use a socket without explicitly converting port to int. When using debug=False, use_reloader is False and Werkzeug calls an inner function that uses make_server. The difference in this behaviour is because BaseWSGIServer uses int(port), explicitly converting the port from str to int.

IMO, if Werkzeug is suposed to allow using a string value in port then we should explicitly convert the port parameter, when possible, and raise an exception when needed. If Werkzeug is not supposed to allow this behaviour, then Flask should allow ints only (and Werkzeug should do it too)


PS: I could make a PR in both repositories if needed. I'm asking because you are maintainers of both projects and I think both projects deserve a good solution 👌

@untitaker
Copy link
Contributor

untitaker commented Mar 25, 2017 via email

@aaossa
Copy link

aaossa commented Mar 25, 2017

But the problem is not in make_server only, a check here won't fix this. An exception in make_server will be raised only when using debug=False, but the behaviour with debug=True will be the same. I think we should add the check in Werkzeug's run_simple.

@untitaker
Copy link
Contributor

Right, forgot about that! run_simple it is then.

untitaker pushed a commit to pallets/werkzeug that referenced this issue Mar 27, 2017
* Check port parameter type at run_simple

Ports should be integers, so a type check is explicitly added to raise
an exception when this is ignored. This inappropiate behaviour was
detected when using `debug=True` in a Flask application, because a
string is accepted and works when it should fail.

Closes pallets/flask#2220

Signed-off-by: Antonio Ossa <[email protected]>

* Add testcase for pallets/flask#2220

This testcase tries to use a string as port parameter, which should not
be allowed by `run_simple`. The test uses `run_simple` directly
because the tests configuration raises the error in a different process
without giving a chance to catch the `TypeError` exception.

See pallets/flask#2220

Signed-off-by: Antonio Ossa <[email protected]>

* Add PR #1088 to Changelog

Signed-off-by: Antonio Ossa <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants