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

String Provider State Params Serialised As Numbers #263

Closed
adamrodger opened this issue Apr 9, 2023 · 8 comments
Closed

String Provider State Params Serialised As Numbers #263

adamrodger opened this issue Apr 9, 2023 · 8 comments

Comments

@adamrodger
Copy link
Contributor

See: pact-foundation/pact-net#449

If a provider state param just happens to contain only numbers then it's serialised to the pact file as a number instead of a string.

You'd expect the pact file to contain:

"providerStates": [
  {
    "name": "state with param",
    "params": {
      "issue": "449" // <-- note that this is a string
    }
  }
]

but it contains

"providerStates": [
  {
    "name": "state with param",
    "params": {
      "issue": 449 // <-- note that this is a number instead of a string
    }
  }
]
@github-actions
Copy link

👋 Hi! The 'smartbear-supported' label has just been added to this issue, which will create an internal tracking ticket in PactFlow's Jira (PACT-937). We will use this to prioritise and assign a team member to this task. All activity will be public on this ticket. For now, sit tight and we'll update this ticket once we have more information on the next steps.

See our documentation for more information.

@rholshausen
Copy link
Contributor

If you are using the function pactffi_given_with_param you need to pass through the value in JSON form. If it is a string value, it must be wrapped in double quotes, i.e.

pactffi_given_with_param(interaction, "state with param", name: "issue", value: "\"449\"")

rholshausen added a commit that referenced this issue Apr 17, 2023
@adamrodger
Copy link
Contributor Author

How come we only have to do that for numbers? Other strings that get passed in don't have quotes around them and thus would be invalid JSON. I'd have to check what Boolean values do but could have a similar problem.

What happens if a regular string has quotes around it? Would those be stripped or would they cause parse errors? That seems pretty unexpected.

@rholshausen
Copy link
Contributor

What happens if a regular string has quotes around it?

If it can't parse it as JSON, it falls back to using the raw string. The problem here is it can parse it as JSON, which is needed in case you actually want to pass numbers and booleans in.

@adamrodger
Copy link
Contributor Author

So if a user passes "foo" or foo then they'll both parse to exactly the same thing, even though the user supplied two different inputs?

That feels like a bit of a foot gun.

Does that mean all FFI consumers need to escape all possible JSON values from user input, just in case it happens to parse as valid JSON when that wasn't intended? That would be a non-trivial task to implement everywhere.

What if they pass something like ["foo"]? Will that parse as an array of strings instead of the literal supplied value?

@rholshausen
Copy link
Contributor

This is not meant to be the format the user provides, that should be an idiomatic value for the language being used. For instance, with Ruby, Python or Groovy the user would be able to pass a map of values, and it should be converted to JSON behind the scenes.

@adamrodger
Copy link
Contributor Author

I'd question if this is really closed as completed. I think it's clear that users could get really weird unexpected behaviour with this.

@YOU54F
Copy link
Member

YOU54F commented May 27, 2024

I'd question if this is really closed as completed. I think it's clear that users could get really weird unexpected behaviour with this.

It would be best to prove or disprove this with tests, and then we have something to work with, as well as some documentation for users.

As to Ron's comment about using json and a commit to update the docstring for usage here

I've implemented that in your issue repro over in pact-net (thank you!), and using the documented suggestion by Ron (parsing as json) works as per your test case.

delta
passing action run

Edge cases are to be expected but happy to unearth them.

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

No branches or pull requests

3 participants