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: Remove base64 conversion #46

Closed
wants to merge 1 commit into from
Closed

fix: Remove base64 conversion #46

wants to merge 1 commit into from

Conversation

2Steaks
Copy link
Collaborator

@2Steaks 2Steaks commented Jul 19, 2024

Why

Removing general base64 conversion to be consistent across the application.

How to test

Response preview should now work on the Generation and Validation view

Notes

Alternatively we keep this and re-base64 media content

Some mime types should be left as base64
@2Steaks 2Steaks self-assigned this Jul 19, 2024
@@ -38,7 +38,8 @@ function parseRequest(request: Entry['request']): Request {
}

function parseResponse(response: Entry['response']): Response {
const content = response.content?.text ? atob(response.content.text) : ''
const content = response.content?.text ?? ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Llandy3d this seems to be similar to one of the ideas you shared, do we also need to change something on the proxy side for this to work?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we can so we should consider the alternative, I will leave another comment

Copy link
Member

@Llandy3d Llandy3d left a comment

Choose a reason for hiding this comment

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

so we will have to leave this because we base64 encode data from the proxy to be able to send it as json (including binary data).

So I don't think it's feasible for us to not expect the base64 encoded content.


proxyData in both Recorder & Validator is straight out of the proxy so it is indeed base64 encoded.

proxyData in the Generator is decoded to be able to work with it, so this is the only broken scenario as far as I'm understanding.
Here we have some options:

  • we can detect if a renderable content is base64 encoded and if not, re-encode it
  • we could decode only the mime-types that we are actually going to work on

I think the first one might be a good approach since the issue is appearing only in rendering, I assume there could be more edge cases we are not considering 🤔

@2Steaks
Copy link
Collaborator Author

2Steaks commented Jul 22, 2024

Closing in favour of #46

@2Steaks 2Steaks closed this Jul 22, 2024
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.

3 participants