Skip to content

Destination plugins receive escaped data #622

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

Closed
candiduslynx opened this issue Jan 24, 2023 · 2 comments · Fixed by #714
Closed

Destination plugins receive escaped data #622

candiduslynx opened this issue Jan 24, 2023 · 2 comments · Fixed by #714
Assignees

Comments

@candiduslynx
Copy link
Contributor

Describe the bug

JSON fields (and possibly others) receive escaped data, as was seen in cloudquery/cloudquery#7001.
The reason for this is that source plugins use json.Marshal that does escaping, but the destination plugins can't perform unmarshaling due to lack of original structure.
So, the data received has unicode characters escaped, and it might be written to the destination in the same way.

Expected Behavior

All values received by destination plugins should be unescaped.

Steps to Reproduce

  1. Create a tagged resource in Azure with key MyKey and value A&B.
  2. Run the destination plugin in debugger
  3. See that the value is received as {"MyKey":"A\u0026B"}, while it should've been presented as {"MyKey":"A&B"}

Possible Solution

Add preliminary unescape for the data in the destination.

Provider and CloudQuery version

latest

Additional Context

No response

@candiduslynx
Copy link
Contributor Author

I suggest adding the Unicode characters to the generated input in the plugin testing for every applicable type.
This way we'll be able to see the data types that require unescaping.

@candiduslynx
Copy link
Contributor Author

Related to cloudquery/cloudquery#7815

@kodiakhq kodiakhq bot closed this as completed in #714 Mar 2, 2023
candiduslynx added a commit that referenced this issue Apr 26, 2023
candiduslynx added a commit that referenced this issue Apr 27, 2023
candiduslynx added a commit that referenced this issue Apr 27, 2023
### Common

Use pointer receivers:
1. For Array types: to have all functions have the same receiver type
(other functions do have pointer receivers, so updating `String()`
method receiver is natural, see `net.IPNet` as a reference.
2. For `Type` implementations: we do return a pointer from `NewXYZType`
function, so it's only natural to define the receiver as pointer as
well.

### `Inet`

#### `InetBuilder`

1. Now accepts `*net.IPNet` (pointer) in `Append`, `UnsafeAppend` &
`AppendValues`
3. `AppendValues` now accounts for `valid` param

#### `InetArray`

1. `String` method uses `%q` format as mainstream Apache Arrow types
2. Added `Value(i int) *net.IPNet` function
4. `GetOneForMarshal` utilizes `Value` implementation

### `JSON`

#### `JSONBuilder`

1. Marshaling with `json.DisableHTMLEscape()` option per #622
2. `AppendValues` now accounts for `valid` param
3. `UnmarshalJSON` implementation is streamlined with helper
`Unmarshal(dec *json.Decoder) error` function (as mainstream Apache
Arrow types)

#### `JSONArray`

1. `String` method uses `%q` format as mainstream Apache Arrow types
2. Added `Value(i int) any` function that will unmarshal the stored data
& return the concrete type
3. Use `json.UnmarshalNoEscape` per #622
5. `GetOneForMarshal` now doesn't perform unmarshaling and, instead,
returns the stored `json.RawMessage`

### `Mac`

#### `MacBuilder`

1. `AppendValues` now accounts for `valid` param

#### `MacArray`

1. `String` method uses `%q` format as mainstream Apache Arrow types
2. Added `Value(i int) net.HardwareAddr` function

### `UUID`

#### `UUIDBuilder`

1. `AppendValues` now accounts for `valid` param
2. Use `uuid.Must` func instead of panic in code

### `UUIDArray`

1. `String` method uses `%q` format as mainstream Apache Arrow types
2. Added `Value(i int) uuid.UUID` func
3. Use `uuid.Must` func instead of panic in code
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.

2 participants