Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
rombert opened this issue Oct 13, 2023 · 12 comments · Fixed by #683 · May be fixed by #3178
Open

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

rombert opened this issue Oct 13, 2023 · 12 comments · Fixed by #683 · May be fixed by #3178

Comments

@rombert
Copy link

rombert commented Oct 13, 2023

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?

@SoulKa
Copy link
Contributor

SoulKa commented Oct 19, 2023

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

@SoulKa
Copy link
Contributor

SoulKa commented Oct 19, 2023

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.

helloanoop added a commit that referenced this issue Oct 22, 2023
fix #571 - allow newlines and encode uri form data
@rombert
Copy link
Author

rombert commented Oct 24, 2023

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.

@SoulKa
Copy link
Contributor

SoulKa commented Oct 26, 2023

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.

@rombert
Copy link
Author

rombert commented Oct 27, 2023

Awesome, thanks @SoulKa !

@helloanoop
Copy link
Contributor

@SoulKa I had to revert the fix as this was causing #1339 and #1076 Commit: a904672

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

@SoulKa
Copy link
Contributor

SoulKa commented Feb 5, 2024

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
Copy link
Contributor

SoulKa commented Feb 5, 2024

@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?

@rombert
Copy link
Author

rombert commented Apr 2, 2024

@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!

@ksimmons-athenahealth
Copy link

For folks looking for a work-around, use a variable for key names that contain colons, e.g. {{resourceTypeKey}} then define the value as "sling:resourceType" in the collection, environment, or request (wherever is most appropriate).

@ash-burns
Copy link

For folks looking for a work-around, use a variable for key names that contain colons, e.g. {{resourceTypeKey}} then define the value as "sling:resourceType" in the collection, environment, or request (wherever is most appropriate).

I can't seem to find a way to use the variable in the Key column, it doesn't seem to resolve it compared to using it in the Value column.
@helloanoop is there a more official way to fix this behaviour yet?

@pietrygamat
Copy link
Contributor

Something like this (#3178) should fix the problem. It builds upon the related change for query parameters (#3045), but I am not how safe are such changes to bru grammar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants