alexdlaird / pyngrok

A Python wrapper for ngrok
https://pyngrok.readthedocs.io
MIT License
421 stars 59 forks source link

Bad tunnel name when serving local directories #50

Closed JennaSys closed 4 years ago

JennaSys commented 4 years ago

Describe the Bug When tunneling a local directory, the generated name in ngrok.py:163: "name": name if name else "{}-{}-{}".format(proto, port, uuid.uuid4()), apparently creates a name that the ngrok api is not able to process correctly, specifically for listing tunnel info or deleting tunnel.

Steps to Reproduce

from pyngrok import ngrok
from time import sleep

ngrok.set_auth_token(SECRET_KEY)
public_url = ngrok.connect('file:///')

print(public_url)
sleep(5)
ngrok.disconnect(public_url)

Above code generates error at disconnect attempt: pyngrok.exception.PyngrokNgrokHTTPError: ngrok client exception, API returned 404: {"status_code":404,"msg":"Not Found","details":{"path":"/api/tunnels/http-file:///-d207771d-59de-4d1b-838c-b56751d14661 (http)"}}

Using the api directly with the generated URI to get tunnel info: http://127.0.0.1:4040/api/tunnels/http-file:%2F%2F%2F-d207771d-59de-4d1b-838c-b56751d14661 results in:

<Error>
<StatusCode>404</StatusCode>
<Message>Not Found</Message>
</Error>

Removing the "port" portion of the name fixes it:

from pyngrok import ngrok
from time import sleep
import uuid

ngrok.set_auth_token(SECRET_KEY)
public_url = ngrok.connect('file:///', name='http-{}'.format(uuid.uuid4()))

print(public_url)
sleep(5)
ngrok.disconnect(public_url)

Expected Behavior ngrok.disconnect() should not generate errors with a valid public_url

Environment

Additional Context N/A

alexdlaird commented 4 years ago

Thanks for raising this. Have added a test (which currently fails, as expected), will patch this shortly and release a new version either later today or early tomorrow. Thanks for bringing it to my attention!

alexdlaird commented 4 years ago

Resolved in v4.1.4, which should be available via PyPI soon.

JennaSys commented 4 years ago

Thanks for the quick turnaround! The good news is that the error on disconnect() seems to be fixed, but it looks like there is still an error from the ngrok api when using the generated tunnel name: http://127.0.0.1:4040/api/tunnels

<tunnelListResource>
<Tunnels>
<Name>http-file%3A%2F%2F%2F-f91b4e56-1659-4cf8-88d5-79014f01ae0b</Name>
<URI>/api/tunnels/http-file%253A%252F%252F%252F-f91b4e56-1659-4cf8-88d5-79014f01ae0b</URI>
<PublicURL>https://3e7fb1fdce72.ngrok.io</PublicURL>
...

http://127.0.0.1:4040/api/tunnels/http-file%253A%252F%252F%252F-f91b4e56-1659-4cf8-88d5-79014f01ae0b

<Error>
<ErrorCode>100</ErrorCode>
<StatusCode>404</StatusCode>
<Message>tunnel not found</Message>
</Error>
alexdlaird commented 4 years ago

Ah, I see. It looks like perhaps the URI being generated is not properly escaping things when it generates the URL, which make actually be an issue with the underlying ngrok implementation that we're surfacing with how we're generating our name field. I'll look in to this more later this week then to see what our proper solution might be—I like port (in this case addr being in the name to distinguish it, but may be more trouble than its worth in this case if I can't find a better solution.

In the meantime looks like you have a workaround by passing the name yourself, but I'll keep you posted!

JennaSys commented 4 years ago

I haven't dug into it quite far enough myself to see specifically what characters are causing the problem (I assume the / ?). Maybe use a character substitution instead of escaping characters? Worst case, since you are generating a UUID for uniqueness anyway, is to just substitute the word "file" for the port instead of the actual location.

alexdlaird commented 4 years ago

Alright, definitely looks to be an issue with ngrok. The / actually was being properly URL encoded before when creating the tunnel, so the right tunnel is made, the right name is even given, but ngrok throws a 404 for it despite giving it the URL ngrok says it correct. I've tried several combinations around that URL too to see where it's encoding is going wrong, but can't figure out how to not make a 404.

Therefore, we'll just mitigate this ourselves and avoid the bug by not putting it in the name if it's for a fileserver, as you suggested.

Both sides of this bug should now be resolved in 4.1.5.

JennaSys commented 4 years ago

Looks good now! Thanks for fixing that so quick.