search

readthedocs / readthedocs.org

The source code that powers readthedocs.org
https://readthedocs.org/
MIT License
8.06k stars 3.59k forks source link

Proxito: inactive versions produce infinite redirect when the HTML exists in the storage #9335

Closed humitos closed 1 year ago

humitos commented 2 years ago

It seems that when a version is active=False but its HTML files exist in the storage, we get an infinite redirect:

$ curl -IL http://docs.reprozip.org/en/0.6.1/
...
curl: (47) Maximum (50) redirects followed

El Proxito should return 404 in these situations.

Reference: https://twitter.com/remram44/status/1536882673821655040 Reference: https://github.com/readthedocs/readthedocs-ops/issues/1139 Reference: https://github.com/readthedocs/readthedocs.org/issues/9109

stsewd commented 2 years ago

This also looks like we failed to remove the files from storage when the version was inactivated.

humitos commented 2 years ago

Yeah, we have a lot of these. They are different problems, tho, and I think we should fix both. However, I'm sure there will be always situations where we fail to delete an inactive version from the storage.

stsewd commented 2 years ago

Looks like we are deleting files when the version is deactivated from the form, but not from the API.

https://github.com/readthedocs/readthedocs.org/blob/a2ca1afe7b590410c76b2df1588ae43305233b0f/readthedocs/projects/views/private.py#L213-L224

Also, we should probably always call the delete function if the version isn't active without checking if the form has changed (just in case the deletion failed, users can just press save).

Also, wonder if we could move this to the save method, but maybe we didn't put it there since it's called too many times (even for an inactive version)?

domdfcoding commented 2 years ago

Presumably related to this issue, I'm seeing an endless redirect for epub download urls:

$ curl -L https://domdf-python-tools.readthedocs.io/_/downloads/en/stable/epub/
curl: (47) Maximum (50) redirects followed

For this project I only have HTML and PDF builds enabled. PDF URL works fine. This project used to have epub builds enabled, but the problem still presents itself for projects where they were disabled from the beginning.

humitos commented 1 year ago

Related to https://github.com/readthedocs/readthedocs.org/issues/9468

stsewd commented 1 year ago

With https://github.com/readthedocs/readthedocs.org/pull/10153, this shouldn't happen anymore. Previously we were raising a 404 (inactive version), then our 404 handler would see that there is an index file and redirect again to the same path (index file redirect).

Now we check that the URL already ends with / and skip the check for the index file.

But we are missing deleting the files from a version when it's deactivated from the API https://github.com/readthedocs/readthedocs.org/issues/9335#issuecomment-1209632300.

stsewd commented 1 year ago

Closing this in favor of https://github.com/readthedocs/readthedocs.org/issues/10221, since the original problem is solved.