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

Don't duplicate Lambda Event Params #184

Merged
merged 1 commit into from
Dec 13, 2024
Merged

Conversation

sternetj
Copy link
Contributor

With version 7.0.1 a query string like ?a=1&a=2&b=3 would produce params like

{
	queryStringParameters: { a: '1,2', b: '3' },
	multiValueQueryStringParameters: { a: ['1', '2'], b: ['3'] },
}

This change causes it to produce params like

{
	queryStringParameters: { b: '3' },
	multiValueQueryStringParameters: { a: ['1', '2'] },
}

The documentation on the API Gateway Event does not provide details on what the expected function of each of these is so I am looking for input as well as proposing one potential solution (this PR). Here are the options as I see it.

  1. Have alpha create events where array values are exclusively in multiValueQueryStringParameters and non-array values are in queryStringParameters
  2. Restore the old behavior of just using queryStringParameters even though that would seem to go against the spec
  3. Update the logic in koa to not allow serverless-http to merge the keys that exist in both multiValueQueryStringParameters and queryStringParameters, i.e. prioritize params from multiValueQueryStringParameters
  4. Something else?

The reason for this change is that patient-service is getting events that look like this

{
  queryStringParameters: {
    _tag: 'http://lifeomic.com/fhir/dataset|1234,http://lifeomic.com/fhir/source|LifeOmic%20Consent'
  },
  multiValueQueryStringParameters: {
    _tag: [
      'http://lifeomic.com/fhir/dataset|1234',
      'http://lifeomic.com/fhir/source|LifeOmic%20Consent'
    ]
  },
}

And when the params are parsed it has the following for ctx.query

{
  _tag: [
    'http://lifeomic.com/fhir/dataset|1234',
    'http://lifeomic.com/fhir/source|LifeOmic%20Consent', 
    'http://lifeomic.com/fhir/dataset|1234,http://lifeomic.com/fhir/source|LifeOmic%20Consent'
  ],
}

This causes the error because it has a tag filter with a , which is not allowed.

@sternetj sternetj force-pushed the avoid-duplicate-query-parmas branch from 0365e6f to 277c206 Compare December 13, 2024 19:06
Copy link
Member

@shawnzhu shawnzhu left a comment

Choose a reason for hiding this comment

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

Thank you! this is a great finding

@sternetj sternetj merged commit 83ff70b into master Dec 13, 2024
6 checks passed
@sternetj sternetj deleted the avoid-duplicate-query-parmas branch December 13, 2024 20:07
Copy link

🎉 This PR is included in version 7.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants