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

Report Files content is assumed to always be text #320

Open
1 task
miguel-ferreira-cko opened this issue Jun 20, 2023 · 5 comments
Open
1 task

Report Files content is assumed to always be text #320

miguel-ferreira-cko opened this issue Jun 20, 2023 · 5 comments
Assignees
Labels
enhancement New feature or request question Further information is requested

Comments

@miguel-ferreira-cko
Copy link

miguel-ferreira-cko commented Jun 20, 2023

Environment

  • Checkout SDK version: 4.0.15
  • Platform and version: .NET 7.0.203
  • Operating System and version: Windows 22H2

Description

Report File content should not always be treated as text. In some cases it can be a binary format (e.g. PDF) which cannot be encoded as a text in .NET (be it ASCII, UTF8...) and must be treated as a byte array / stream / base64 so that it can be properly stored.

Expected behavior

Report File content accurately contains the original file bytes.

Current behavior

Report File content contains a string encoding of the original file bytes, which when encoded back into binary form will be missing data.

Steps to reproduce

  1. Download the PDF file via command line (e.g. PowerShell) Invoke-RestMethod -Method Get -Uri 'https://api.checkout.com/reports/REPORT_ID/files/FILE_ID' -Headers @{"Authorization"="SECRET"} -OutFile original.pdf
  2. Download the same PDF file using the Checkout SDK and attempt to save in any of the available text encodings
using Checkout;
using System.Text;

ICheckoutApi api = CheckoutSdk.Builder().StaticKeys()
    .SecretKey("SECRET")
    .Environment(Checkout.Environment.Production)
    .Build();

var reportId = "REPORT ID WHERE .Files[0].Format == PDF";

var report = await api
    .ReportsClient()
    .GetReportDetails(reportId);

var file = await api
    .ReportsClient()
    .GetReportFile(reportId, report.Files[0].Id);

File.WriteAllBytes("report.pdf", Encoding.Unicode.GetBytes(file.Body));
File.WriteAllBytes("report2.pdf", Encoding.UTF8.GetBytes(file.Body));
File.WriteAllBytes("report3.pdf", Encoding.ASCII.GetBytes(file.Body));
File.WriteAllBytes("report4.pdf", Encoding.UTF32.GetBytes(file.Body));
File.WriteAllBytes("report5.pdf", Encoding.Latin1.GetBytes(file.Body));
File.WriteAllText("report6.pdf", file.Body, Encoding.ASCII);
  1. Observe that none of the PDF files that had their bytes text encoded previously is keeping the original file bytes:
    image

Possible solution

  • I may be able to implement this bug fix

Additional information

@armando-rodriguez-cko
Copy link
Contributor

Thank you @miguel-ferreira-cko, we will review the information you provided shortly and get back to you to resolve this issue.

@a-ibarra
Copy link
Contributor

@miguel-ferreira-cko We raised this issue time ago, the thing is that this endpoint is a redirect, and previously this endpoint was always returning a string, in worst case we should return only the string of the redirect and merchant should handle the type of the content based on the response.

@miguel-ferreira-cko
Copy link
Author

@armando-ibarra-cko this endpoint has always returned 302 Redirect into a pre-signed location within AWS S3 that downloads a file. The HttpClient in .NET follows redirect links by default.

The response type of GetReportFile on the SDK could simply be the HttpClient Response itself

@armando-rodriguez-cko
Copy link
Contributor

Thank you @miguel-ferreira-cko for the information, we will review and we will try to improve this point. We will let you know when we have something.

@armando-rodriguez-cko armando-rodriguez-cko self-assigned this Aug 17, 2023
@armando-rodriguez-cko armando-rodriguez-cko added enhancement New feature or request question Further information is requested labels Aug 17, 2023
@armando-rodriguez-cko
Copy link
Contributor

@miguel-ferreira-cko can you please confirm that this behavior is no longer happening?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Development

No branches or pull requests

3 participants