search

shlinkio / shlink

The definitive self-hosted URL shortener
https://shlink.io
MIT License
3.13k stars 253 forks source link

Allow "&" and "+" characters in custom slugs again. #2156

Open tiritibambix opened 2 months ago

tiritibambix commented 2 months ago

Summary

Hi there!

I'm a big fan of shlink and have been using it for over a year now. It's been a great tool for me, super efficient and convenient.

I do have a small request though - I used to be able to use characters like "&" and "+" in my custom slugs, which was really handy for readability. It would be awesome if I could use them again.

I receive this error:

Provided data is not valid

Invalid elements: [customSlug]

Thanks so much for considering this!

Use case

/T+B-lunch /T&C_car

acelaya commented 2 months ago

I used to be able to use characters like "&" and "+" in my custom slugs

In what version did you stop being able to use those? I don't remember making a conscious decision to "not allow" them, so it might be a side effect of some other change.

In any case, it might be a desirable side effect, as those are invalid URL characters that would need to be encoded, but with proper url-encoding, I don't see an immediate reason.

Also, can you elaborate what are you experiencing that you are interpreting as "not being able to use them"? Do you get an error? If so, which one? Have you checked the logs?

acelaya commented 1 month ago

Closing until requested information is provided.

tiritibambix commented 1 month ago

Thanks for your answer.

I am away From home for quite a while so I'm sorry I can't answer properly for the moment as I don't have access to the logs.

I can't remember what version it started not working.

As for the error message, it is mentioned in my original post.

acelaya commented 1 month ago

As for the error message, it is mentioned in my original post.

Thanks! I somehow missed that, perhaps I read the message in the email notification from GitHub and didn't realize it had been updated right after.

acelaya commented 1 month ago

Ok, so I found when was this introduced. It was the result of this bug report https://github.com/shlinkio/shlink/issues/1901 (you can ignore everything after my first comment there, because the conversation deviated into a feature request not related to the bug itself).

To fix that, this PR was provided, which validates custom slugs to disallow URL-reserved characters as per this RFC.

I need to do a bit of investigation and think on potential side effects of changing this, or if that decision was wrong in the first place.

tiritibambix commented 1 month ago

I'm really glad you're considering looking into this and I'm looking forward to reading you on this matter :)