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

fix(iRest): Use encodeURIComponent for url params #72

Closed
wants to merge 1 commit into from

Conversation

amagid
Copy link
Contributor

@amagid amagid commented Jul 16, 2019

encodeURI() does not encode &, #, or ? characters, so if they are
present anywhere in the submitted XML document, they will cause the URI
to be invalid and result in a 400 error. Instead, using
encodeURIComponent() on each individual URL param will safely encode all
possible characters.

Closes #71

encodeURI() does not encode &, #, or ? characters, so if they are
present anywhere in the submitted XML document, they will cause the URI
to be invalid and result in a 400 error. Instead, using
encodeURIComponent() on each individual URL param will safely encode all
possible characters.

Signed-off-by: Aaron Magid <ahm64@case.edu>
@abmusse
Copy link
Member

abmusse commented Jul 18, 2019

Docs for encodeURIComponent.

@amagid

These changes look good.

Can you provide a simple example that shows the failure of encodeURI and the fix encodeURIComponent provides when using itoolkit?

This will be helpful in creating a test case.

Thanks for contributing!

Copy link
Member

@abmusse abmusse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@abmusse abmusse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed this PR is intended for Master branch we should put this on ice until PR #78 Lands.

@amagid
Copy link
Contributor Author

amagid commented Aug 20, 2019

Hey, so sorry - I completely missed your message! Simplest way to test this is the way I ran into it - by running a SQL query which contained an '&'. XMLService will return a 400 error without this fix.

Simple test:

const query = `select * from test_table where name='&#!test??'`;
const sql = new xt.iSql();
sql.addQuery(query);
sql.fetch();
sql.free();
conn.add(sql);
conn.run(xmlResponse => console.log(xmlResponse));

@abmusse abmusse self-requested a review October 2, 2019 18:30
Copy link
Member

@abmusse abmusse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amagid

After merging PR #78,

Looks like this PR needs to be updated because lib/irest.js has been moved and changed.

This PR should be updated with changes like the following in lib/transports/irest.js:

let xmlEnc = `db2=${encodeURIComponent(database)
  }&uid=${encodeURIComponent(username)
  }&pwd=${encodeURIComponent(password)
  }&ipc=${encodeURIComponent(ipc)
  }&ctl=${encodeURIComponent(ctl)
  }&xmlin=${encodeURIComponent(xmlInput)
  }&xmlout=${encodeURIComponent(outputBuffer.toString())}`;

kadler added a commit that referenced this pull request Jan 13, 2020
encodeURI() does not encode &, #, or ? characters, so if they are
present anywhere in the submitted XML document, they will cause the URI
to be invalid and result in a 400 error. Instead, using
encodeURIComponent() on each individual URL param will safely encode all
possible characters.

Thanks to @amagid for the original PR.

Obsoletes #72
Fixes #71

Co-authored-by: Aaron Magid <ahm64@case.edu>
abmusse pushed a commit that referenced this pull request Jan 17, 2020
encodeURI() does not encode &, #, or ? characters, so if they are
present anywhere in the submitted XML document, they will cause the URI
to be invalid and result in a 400 error. Instead, using
encodeURIComponent() on each individual URL param will safely encode all
possible characters.

Thanks to @amagid for the original PR.

Obsoletes #72
Fixes #71

Co-authored-by: Aaron Magid <ahm64@case.edu>
@abmusse
Copy link
Member

abmusse commented Jan 17, 2020

Obsoleted by #89

@abmusse abmusse closed this Jan 17, 2020
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 this pull request may close these issues.

Bug handling non-url-safe characters in XML document
3 participants