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

Webhook notifications URL size #3669

Closed
jonathansberry opened this issue Feb 23, 2018 · 5 comments
Closed

Webhook notifications URL size #3669

jonathansberry opened this issue Feb 23, 2018 · 5 comments
Labels
Bug A bug
Milestone

Comments

@jonathansberry
Copy link

jonathansberry commented Feb 23, 2018

Details

Expected result:

I want to setup a webhook notification to our software when a readthedocs build fails. I have a url which should do the trick. When I visit it with a simple GET request in a browser the notification works perfectly. I want to submit the URL as a webhook notification at the page in the details above.

Actual Result

When I try to submit it as a webhook notification at the page above I get an ascii whale and the error message "Fail. Check back in a bit!". Some playing around has made me think that my webhook URL is too long, as it is accepted if I cut parts out of it. However, the URL is only approx 530 characters long, which whilst being long, should be acceptable I would think. It is long purely because it has to include a signed access token. If I cut the access token out, the url is accepted. Here is a fairly good mockup of the webhook URL I am trying submit, including a fake access token:

https://blah.blah.blah.info/notify?message=message_here&subject=DOCS%20FAILIURE&access_tok=DgyppMOXVVJY1I6JlTKFxl7zf2NH0Z8lwcZyy9NlSJnrmZ4AinNfn7nSHGjqGVPiU6UNzjJxDsVSP2IK8tBO-iHjIY2c47ijiN0c5J3Gpj4sPwmns8kWyLLMlNsP0Az29ZOwl3VBAm317jkFiijWJ5bDy.RG3F8Ztdt.20qM7rimwRBxIdQUg6h0-z9iywWK_Cp8DUfiyCAy0KzFgfTuVRJD4YeS57kKDqG_RB6bd_Vtj2dHvOilGmCjMLay9JeCp60xX3Er7PyGMVMcxUMh11xowGFMmhYFVWlOkk0MHLxaG2Ocmgok9I1VHRQbkDN9uJWX2E-vewf84XwGTsiTH-eo3pWiU-c7mweMPF3t9C8XlpHJ1ofpl1dSeqspzHYaDIYvkyhgTxefSNJMuIb2eF1HUQExCyFg22ibbTszDRttRnP01AO

Are the folk at readthedocs aware of this, or able to explain why the URL won't be accepted? I can't find anything in your docs about it.

Many thanks.

@stsewd
Copy link
Member

stsewd commented Feb 23, 2018

I can't reproduce this on my local instance, but I checked on the .org site and indeed I can see the error page (500 error). Looking the docs for Charfield (also used by UrlField), there isn't a max length, perhaps this is setting up in production? @humitos can you confirm this?

@humitos
Copy link
Member

humitos commented Feb 26, 2018

@stsewd yeah, the same here.

The max_lenght by default is 200 (https://docs.djangoproject.com/en/2.0/_modules/django/db/models/fields/#URLField)

So, it should fail when validation the URL. Which is not doing for some reason at https://github.com/rtfd/readthedocs.org/blob/0e1112f96e3ba344271a44305028a811072fd10a/readthedocs/projects/forms.py#L485-L488

In production (PostgreSQL) it fails with the proper error:

DataError: value too long for type character varying(200)

but I don't know why it doesn't fail in development (SQLite).

I think the solution here is:

  • make the validation form to work
  • increase the lenght of the URL to something bigger

@humitos humitos added the Bug A bug label Feb 26, 2018
@chirathr
Copy link
Contributor

chirathr commented Feb 26, 2018

Hi @humitos I am new here, can I work on this issue?

From your description, I understood that the max_length is to increased(maybe 600 characters?) and there should be a form validation error if it goes beyond that.

@RichardLitt
Copy link
Member

That sounds right to me, @Chirath02. We can work out more details in your PR (#3680).

@agjohnson agjohnson added this to the Admin UX milestone Jun 1, 2018
@stsewd
Copy link
Member

stsewd commented Nov 28, 2018

This was already done in #3680 (merged)

@stsewd stsewd closed this as completed Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug
Projects
None yet
Development

No branches or pull requests

6 participants