bruno icon indicating copy to clipboard operation
bruno copied to clipboard

fix: Fix name validation consistency

Open Its-treason opened this issue 2 years ago • 20 comments

Edit: Updated the PR, please read next comment

With this PR I want to fix validation problems when creating folder, requests or environments. All names now use a validation regex:

/^(?!CON|PRN|AUX|NUL|COM\d|LPT\d|^ |^\-)[\w\-\. \(\)\[\]]+[^\. ]$/

This regex ensures various rules when naming files on the file system, so users don't run into weird problems:

  • Only allowing certain characters: a-z, A-Z, 0-9, _, -, ., (, ), [, ] and (Space) to ensure file name compatibility between operating and file systems
  • Disallowing special cases, e.g., Windows file names must not end with a .
  • Disallowing Windows Device names e.g., CON, NUL

You can test this regex on regexr: https://regexr.com/7l552

For reference for all limitations, I used:

  • https://stackoverflow.com/questions/1976007/what-characters-are-forbidden-in-windows-and-linux-directory-names
  • https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words

I also removed the is-valid-path/isValidPathname check from the collection creation as this check was very strict and would not pass with names containing common special characters and a failed checked would only fail with an “An error occurred” toast. This should resolve #174


For the future we should make it, so all characters are allowed in the Request/Folder name, but the special characters are only saved in the .bru file meta and the file name is automatically stripped from special characters. This should then resolve: #189 & #388. But will take some time to fix as it is more complicated.

Its-treason avatar Oct 04 '23 15:10 Its-treason

Somewhat related to #365

lared avatar Oct 04 '23 18:10 lared

I updated my PR to allow all special chars in request names, this should resolve #388, #189 & #174. I also updated the name length validation to 250 characters, 256 characters is the windows length limit, so 250 leaves some space for the file extension, this should resolve: #163, #378 & #501.

Example:

https://github.com/usebruno/bruno/assets/39559178/740c0503-cbcf-48fd-8c3e-518394d5554c

Example with old project, created with the current bruno version:

https://github.com/usebruno/bruno/assets/39559178/77cebae8-93d0-4d86-bd17-51803970527e

There might still some things to fix, I look into that tomorrow

Its-treason avatar Oct 06 '23 20:10 Its-treason

Hey @helloanoop, this PR should be ready to be reviewed and merged now. After this, I can start working on updating the Postman/Insomnia importer to close #365

Its-treason avatar Oct 08 '23 16:10 Its-treason

I update the sanitization Regex to match the directory regex. I noticed that it would be very hard for a language like Japanese to use Bruno with the other regex, because こんにちは & ありがとう would both be converted to _____.bru so only one would be allowed.

Its-treason avatar Oct 14 '23 17:10 Its-treason

I think { and } should also be added to the regex as they are frequently used in postman collection and are allowed on unix/windows

/^(?!CON|PRN|AUX|NUL|COM\d|LPT\d|^ |^\-)[\w\-\. {}\(\)\[\]\!]+[^\. ]$/

nomorsug avatar Oct 18 '23 13:10 nomorsug

I think { and } should also be added to the regex as they are frequently used in postman collection and are allowed on unix/windows

/^(?!CON|PRN|AUX|NUL|COM\d|LPT\d|^ |^\-)[\w\-\. {}\(\)\[\]\!]+[^\. ]$/

Hey @nomorsug, sorry for the confusion, but the Regex in my initial comment is not up-to-date anymore.

With the newest changes, you can use any character in the request name. To make sure filenames are valid, characters like ? or / will be replaced with _, but this only affects the filenames.

Its-treason avatar Oct 18 '23 18:10 Its-treason

@Its-treason was talking about the folder names, this regex to be specific packages/bruno-app/src/utils/common/regex.js to allow for curly braces {}

nomorsug avatar Oct 18 '23 20:10 nomorsug

@nomorsug Sorry, I forget about the directory regex. I updated it to match the request sanitization regex:

/^(?!CON|PRN|AUX|NUL|COM\d|LPT\d|^ |^\-)[^<>:"/\\|?*\x00-\x1F]+[^\. ]$/

This now disallows only the problematic special characters like ?, < & some other windows related stuff, while allowing {, } and much more like Japanese characters.

Thank you for pointing that out!

Its-treason avatar Oct 18 '23 22:10 Its-treason

@Its-treason scanning through the changes in this PR, it seems that "sanitize" and "filename" are misspelled in many places and in many different ways. Can you do a cleanup pass to fix these typos?

cardonator avatar Oct 18 '23 22:10 cardonator

@helloanoop can we get this PR reviewed/merged? It will dramatically improve file handling for environments.

cardonator avatar Oct 24 '23 17:10 cardonator

@helloanoop any chance of this getting merged soon? Postman imports are still impacted by the incorrect validation and v1.0 has been released

nomorsug avatar Oct 31 '23 14:10 nomorsug

@helloanoop any chance of this getting merged soon? Postman imports are still impacted by the incorrect validation and v1.0 has been released

That would be very nice. I'm also struggeling with the fact that i cannot use "/" in my request names.

fzberlin23 avatar Jan 19 '24 13:01 fzberlin23

Also looking forward to this being merged! :) ..this as been a show stopper for me since the Insomnia drama image

fractalf avatar Feb 02 '24 08:02 fractalf

Really needing this to get into Bruno to make it more usable (also using / in request names)

wouter-leistra avatar Mar 21 '24 05:03 wouter-leistra

Really needing this to get into Bruno to make it more usable (also using / in request names)

Not sure when, or if this will ever be merged into Bruno. For now you can use my fork: https://github.com/Its-treason/bruno/releases/tag/nightly it's compatible with mainline Bruno and currently used by our Team, because it includes some performance improvements.

Its-treason avatar Mar 21 '24 09:03 Its-treason

Please fix this. This stopping me from switching to bruno.

trbtm avatar Jun 14 '24 14:06 trbtm

Showstopper for my team too. @helloanoop

bremyozo avatar Jul 06 '24 20:07 bremyozo

I also wanted to switch to bruno, so I started with a sample request (/products), renamed the request (/products/id), and I get error. (macOS, latest version).

This is a critical bug in my opinion and it's a blocker for me to switch to this app.

daniel-tirzuman avatar Aug 02 '24 12:08 daniel-tirzuman

For those tired of waiting for this and sick of Insomnia...

I found another lightweight (based on electron) client here: https://github.com/flawiddsouza/Restfox

fractalf avatar Aug 18 '24 15:08 fractalf

For those tired of waiting for this and sick of Insomnia...

I found another lightweight (based on electron) client here: https://github.com/flawiddsouza/Restfox

Or checkout my fork: https://github.com/Its-treason/bruno/releases/tag/nightly it includes this and many more fixes while being fully compatible with Bruno.

Its-treason avatar Aug 18 '24 16:08 Its-treason

Hey everyone!

Apologies for such a late update on this. In hindsight, we should have prioritised and delivered a solution for this earlier. We have been working on this issue over the last week.

The core solution in this PR is around normalizing the filename and replacing them with _.

const sanitizeFilename = (name) => {
  return name.replace(/[<>:"/\\|?*\x00-\x1F]/g, '_');
};

We are working on an improvisation on top of this PR that allows you to have more control over the filename under which the request data is stored. Here is the PR - #3094 tracking this work.

Our goal is to have this refined and delivered within the next two weeks. Thanks for your patience.

https://github.com/user-attachments/assets/bb71c207-8335-431e-b930-9552f7501b7f

helloanoop avatar Sep 15 '24 10:09 helloanoop

@helloanoop @lohxt1 any updates or timeline for this?

trbtm avatar Oct 23 '24 14:10 trbtm

I was reporting this bug (like accepting a / in the name) in Discord recently as well.. This PR might just solve everything and is isn't hard to implement this.

melroy89 avatar Nov 02 '24 10:11 melroy89

We have finally released support for Custom Filename and Folder Name.

You can now save request names containing any character, including special ones like /, \, [, ], * etc. For filesystem compatibility, we automatically replace unsupported characters with a - in filenames, while keeping the request name intact.

Additionally, you can easily override the auto-generated filenames and folder names with your own custom choices, as illustrated in the screenshot.

@Its-treason @lared @nomorsug @cardonator @fzberlin23 @fractalf @wouter-leistra @trbtm @bremyozo @daniel-tirzuman @melroy89 We'd love to hear your feedback! If you have any thoughts or encounter additional issues, please share them with us in issue #2996.

@Its-treason We've also included the regex here alongside tests

@Its-treason Closing this PR. If you feel we could further improve on the naming stuff, please create a new PR.

image

helloanoop avatar Mar 19 '25 07:03 helloanoop

Great! Will test it soon. Assuming this will be part of the new upcoming release?

melroy89 avatar Mar 19 '25 10:03 melroy89

Yeah seems to work correctly for our use cases. Now I just hope that the sorting will be fixed so not only the folders are sorted in put the a-z or z-a mode but also the requests within the folders.

wouter-leistra avatar Mar 19 '25 11:03 wouter-leistra

@wouter-leistra Yes, folder sorting/sequencing is on our roadmap to be delivered in April @melroy89 Its released. You can download it here

helloanoop avatar Mar 19 '25 11:03 helloanoop

@wouter-leistra Yes, folder sorting/sequencing is on our roadmap to be delivered in April @melroy89 Its released. You can download it here

I do use the deb repo. http://debian.usebruno.com/ don't forget to update these as well, in case you didn't yet 😅

melroy89 avatar Mar 19 '25 15:03 melroy89

Works. Finally. Thanks!

fractalf avatar Mar 20 '25 21:03 fractalf

@melroy89 We've now updated debian package.

helloanoop avatar Mar 24 '25 15:03 helloanoop