reminiscence
reminiscence copied to clipboard
Special characters in folder name break the folder
Summary
Some URL special characters (&, /, etc.) are allowed in folder names, and render the folder inaccessible.
Environment
Version: Release v0.1
Python: Anaconda 3.6.5
Set up using Normal Method on Debian Stretch
Running gunicorn behind Nginx as described in README.md
Reproduction Steps
- Create a new folder with an alphanumeric name (e.g.
Recipes) - Add URLs to the folder
- From Home, rename the folder (Select -> Rename)
- Enter a name which contains a forward slash (
/) or ampersand (&) (e.g.Recipes & Cooking) - Observe the rename succeeding, and the new name appearing properly on the Home page
- Attempt to open the folder to view the URLs inside
Expected Behaviour
The folder should open as expected, and the special characters should be properly escaped in the page URL
Observed Behaviour
The page fails to load (HTTP 404), as the special characters present in the URL are not escaped and are invalid (e.g. Recipes & Cooking above tries loading as https://myserver.example/myusername/Recipes & Cooking)
Recovery
I was able to repair my installation by editing the database manually and replacing all instances of the invalid folder name with a valid one in the directory column of the page_library table.
Notes
It would be nice if these characters were allowed (and properly escaped so the page continues to load), but if this is infeasible then they should be disallowed in the folder name field to prevent breaking an installation.
I'd be happy to dig in to the project and contribute a pull request if you'd like. Thanks for all your work on this project so far, it's awesome!
Forward slash ( '/' ) is certainly not allowed at the moment, since it will break url path. But it is possible to include '&', by changing url regex pattern. You are free to make changes and submit pull request with small test case.
By the way, about recovery process, I think, it is possible to rename invalid names from web-interface itself instead of manually editing database.
For a time being, I'll add note on invalid characters in the README.
this project so far, it's awesome!
It's good to hear that people are finding this project useful.
Thanks for the response. I just want to confirm that I tried renaming an invalid folder, and it doesn't work because the rename (.../myusername/Recipes & Cooking/rename) and remove (.../myusername/Recipes & Cooking/remove) URLs also 404. It would probably be worthwhile to check for special characters on submit and display an error message until they can be properly supported.
This is strange.
As per urlpattern, 'Recipes & Cooking/rename' is completely allowed. I just checked for both rename and remove urls. Both are working completely fine.
Can you post version of django? Reminiscence works properly only with django v2.0+
The result of pip freeze in my virtual environment is:
aiohttp==3.3.2
amqp==2.3.2
async-timeout==3.0.0
attrs==18.1.0
beautifulsoup4==4.6.3
billiard==3.5.0.4
bs4==0.0.1
celery==4.2.1
chardet==3.0.4
cssselect==1.0.3
Django==2.1
django-widget-tweaks==1.4.2
gunicorn==19.9.0
idna==2.7
idna-ssl==1.1.0
kombu==4.2.1
lxml==4.2.4
multidict==4.3.1
nltk==3.3
psycopg2==2.7.5
pytz==2018.5
readability-lxml==0.7
redis==2.10.6
six==1.11.0
vine==1.1.4
yarl==1.2.6
Django version is fine. But I'm not able to reproduce the bug related to rename/remove!
By the way I've added support for '&' in devel branch.
Supporting every special character including '/' will require lot of changes in codebase including urlpatterns.
Let me know if there's any additional logs I can submit to help reproduce!
One suggestion for allowing all special characters is to use slugs, and store it along side the "pretty" title. Use the slug in the URL, but the pretty title in the list of folders shown to the user. You can automatically generate the slug from whatever title the user chooses. I'll work on this and put up a PR if you're interested.
Alternatively, I think just blocking invalid characters from being submitted as a valid name would help a lot. Perhaps the built in Regex Validator could be used.
Also glad to see '&' is now supported, that's a big one for me.