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] Extracting substring to token value doesn't reference tag properly #1809

Closed
ronkot opened this issue Nov 26, 2019 · 8 comments · Fixed by #1863
Closed

[Bug] Extracting substring to token value doesn't reference tag properly #1809

ronkot opened this issue Nov 26, 2019 · 8 comments · Fixed by #1863

Comments

@ronkot
Copy link

ronkot commented Nov 26, 2019

I'd need to chain certain requests. In the middle of the chain I'd need to parse a substring from a response JSON field and use that value in the next step. I couldn't find out way to do this. Is it possible somehow?

Example:

Response body

{
  "url": "https://redirect.to?code=abc123"
}

And I'd like to extract the code query param value from the $.url field.

@welcome
Copy link

welcome bot commented Nov 26, 2019

👋 Thanks for opening your first issue! If you're reporting a 🐞 bug, please make sure
you include steps to reproduce it. If you're requesting a feature 🎁, please provide real
use cases that would benefit. 👪

To help make this a smooth process, please be sure you have first read the
contributing guidelines.

@gschier
Copy link
Contributor

gschier commented Nov 26, 2019

You'll have to store the response value in a temporary environment variable first.

For example:

{
  "url": "<RESPONSE TAG>",
  "code": "{{ url | replace('https://redirect.to?code=', '') }}"
}

Is this for OAuth 2.0? If so, have you tried using Insomnia's built-in OAuth 2.0 support?

@ronkot
Copy link
Author

ronkot commented Nov 27, 2019

Thanks @gschier, nice idea 👍 I tried this approach but unfortunately it doesn't work fully. When I create the temporary code environment variable in Manage Environments, the extracted code value seems ok. But when I use the code environment value in a request, the value that gets injected is the original url (not the extracted code). I try to play around more to see if it's just PEBCAK.

Is this for OAuth 2.0? If so, have you tried using Insomnia's built-in OAuth 2.0 support?

Yes it is. I tried also the built-in OAuth 2.0 flow, but unfortunately it seems that our OAuth flow does not fully follow the standard (shame). Hopefully we'll get that fixed in the future!

@ronkot
Copy link
Author

ronkot commented Nov 28, 2019

I tried to debug my configuration but couldn't make it work. Tested that when I change the url environment variable to be a string constant everything works as expected. But when I use the response tag method you suggested the code environment value that is used in request is plain url value. Looks like the the replace filters are not run for response tags.

Could this be a problem in insomnia?

@gschier
Copy link
Contributor

gschier commented Dec 4, 2019

Okay, I just reproduce this. Posting a reproducible export here for reference:

Run in Insomnia}

@gschier gschier changed the title [Question] Extracting substring to token value [Bug] Extracting substring to token value doesn't reference tag properly Dec 4, 2019
@develohpanda
Copy link
Contributor

I spent some time investigating this one, and here are my findings. There isn't an issue with how a variable is resolved for a request, there is a more fundamental issue with resolving variables, because I managed to reproduce this bug within the environment variable editor.

If the piped variable is coming from a block using insomnia-plugin-response, we see this behavior

{
  "url": "{% response 'body', ... %}", // returns https://redirect.to?code=123
  "code": "{{ url | replace('https://redirect.to?code=', '') }}" // returns 123
  "use-code": "{{ code }}" // returns https://redirect.to?code=123
}

And if the piped variable is coming from a regular string, we see this behavior

{
  "url": "https://redirect.to?code=123", // returns https://redirect.to?code=123
  "code": "{{ url | replace('https://redirect.to?code=', '') }}" // returns 123
  "use-code": "{{ code }}" // returns 123
}

It seems there is an issue with nunjucks, when nesting variable <- variable <- block, while variable <- variable <- variable works.

I can see some interesting behavior inside the templating.render, when we call the nunjucks renderString method, but I'm going in circles at this point and need to bounce some ideas I think.

const nj = await getNunjucks(renderMode);
nj.renderString(text, context, (err, result) => {
if (err) {
const sanitizedMsg = err.message

@gschier
Copy link
Contributor

gschier commented Dec 20, 2019

Thanks for taking a look @develohpanda! Seeing your example environments made me remember that this is in the codebase:

/**
* Sort the keys that may have Nunjucks last, so that other keys get
* defined first. Very important if env variables defined in same obj
* (eg. {"foo": "{{ bar }}", "bar": "Hello World!"})
*
* @param v
* @returns {number}
*/
function _nunjucksSortValue(v) {
if (v && v.match && v.match(/({%)/)) {
return 3;
} else if (v && v.match && v.match(/({{)/)) {
return 2;
} else {
return 1;
}
}

Basically, it ensures that variables are rendered before template tags because I figured the most common use case would be to pass a variable into a tag instead of the other way around. In hindsight, my assumption might have been backwards. Either way, sorting either breaks one use case or the other.

But I just had a thought. Now that we have merged your PR to preserve sort order of environment properties, we probably don't need to sort anymore. We should be able to rely on the user to define variables in preferred render order (top-to-bottom).

@develohpanda
Copy link
Contributor

develohpanda commented Dec 21, 2019

Thanks @gschier, that was the pointer I needed!

Either way, sorting either breaks one use case or the other.

I'm concerned about changing the sorting because this is likely to introduce breaking changes, but it is also a fairly specific use case.

I've written a breaking test for this and updated the logic to validate it fixes the behavior, which it does, to an extent. Now, both tag ({%) and variable ({{) nunjucks are given the same priority, which means if they are run in the user-defined order, they work properly.

However... the PR around preserving sort-order only kicks in while rendering the keys, and doesn't impact the order in which keys are processed through the rest of the code-base. That object loaded from the db stays as-is.

While debugging, I set the keys in a particular order, but they are being processed in alphabetical order:
image
image

But if I rename the keys to ensure they are processed in the correct order, things work.
image
image

The direction I will go to fixing this up will be to re-order the environment.data object, but where this re-order happens is TBC. I'm thinking that we re-order immediately after loading from the database, but this could introduce regressions and I'm leaning towards ordering only when the render context is being created. I will be publishing a new version of json-order to achieve this.

Let me know if you have any thoughts, specifically around introducing a potentially breaking change! 😄

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

Successfully merging a pull request may close this issue.

3 participants