bruno icon indicating copy to clipboard operation
bruno copied to clipboard

Bug: unable to set request bodies with colon characters in their names

Open rombert opened this issue 2 years ago • 9 comments

Hi,

I work on applications that make heavy use of namespacing property names with colons, e.g. sling:propertyType. I imported one collection for such an app from Insomnia and noticed that the parameters are displayed incorrectly in the UI.

image

The data is stored in the file as

body:form-urlencoded {
  message: The Apache Sling REST API is awesome!
  createdBy:
  sling:resourceType: pospai/component/comment
  sling:redirect: /pospai/home/welcome/jcr:content/comments.html
}

Naively trying to escape the colon values did not work, both

  sling\:resourceType: pospai/component/comment

and

  "sling:resourceType": pospai/component/comment

look for the first : character to identify the field name.

Is there a way to have field names include the colon character?

rombert avatar Oct 13 '23 14:10 rombert

The issue is not the colon, but rather the newline. I'll investigate on how newlines can be used in the request.

SoulKa avatar Oct 19 '23 18:10 SoulKa

The issue was that the imported (and also any manually written) parameter values have not been encoded thus resulting in invalid bruno files. Fix is created. With the fix, everything should work after you reimport the collection.

SoulKa avatar Oct 19 '23 19:10 SoulKa

Thanks for the fix @SoulKa ! With version 0.27.2 I can manually adjust the fields after the import so that the property is named sling:resourceType, but the initial import is still incorrect. While my immediate problem is solved, I've attached my Insomnia export - Insomnia_2023-10-13.json in case there is interest in fixing the import as well.

You can look at home Create comment (John Doe) is imported, for instance.

rombert avatar Oct 24 '23 10:10 rombert

Thanks for your collection! I could recreate your issue and created a fix for it. When importing the collection, the parameters now look as expected:

grafik

This actually affected not only the form-data parameters, but any place where you can enter keys and values.

SoulKa avatar Oct 26 '23 19:10 SoulKa

Awesome, thanks @SoulKa !

rombert avatar Oct 27 '23 12:10 rombert

@SoulKa I had to revert the fix as this was causing #1339 and #1076 Commit: https://github.com/usebruno/bruno/commit/a90467255535ba3c740363d5aa226bbede8b14ee

This fix is going in the 1.8.0 release.

So how do we solve this issue

Before we approach a solution, I think we should never auto encode / decode values. It should always be configurable (See #732)

I see 2 Solutions

Toggle Inside Request Settings

I am thinking of supporting a new tab inside the request pane called Settings. This should have a toggle that allows to choose to encode Form URL Params before saving the request.

image

Bru Lang

There is a standardization effort ongoing for Bru Lang. The new lang structure will support multiline strings and this issue would go away. ETA for support for migrating to new structure is likely early-march 2024

helloanoop avatar Feb 05 '24 19:02 helloanoop

I think adjusting the Bru Lang is the correct way to go in the long term. I haven't checked all the Bruno updates since then. Since the Bru Lang will support multiline strings, is it also safe for other edge cases like :, ", and % signs? I have noticed issues with a colon : in other parts of bruno, too. When standardizing the Bru Lang, it would be good to have a way to escape all problematic symbols.

SoulKa avatar Feb 05 '24 19:02 SoulKa

@helloanoop I believe we can close the PR #804 (fix for #802) then? The issue will be resolved when the Bru Lang supports multiline strings right?

SoulKa avatar Feb 05 '24 19:02 SoulKa

@helloanoop - I was looking around the issue tracker and discussions and could not find a place to watch for

There is a standardization effort ongoing for Bru Lang.

Is that already available somewhere? Thanks!

rombert avatar Apr 02 '24 12:04 rombert