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

[FFI] with_body and with_binary_file confusion #336

Closed
JP-Ellis opened this issue Nov 10, 2023 · 4 comments
Closed

[FFI] with_body and with_binary_file confusion #336

JP-Ellis opened this issue Nov 10, 2023 · 4 comments
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@JP-Ellis
Copy link
Contributor

Summary

The FFI provides two functions to set the expected body of an interaction:

Both function require the body to be passed as a char* pointer to an array, but they differ in that (at least according to the documentation):

  • with_body requires the interaction contents to be either a NULL pointer, or point to valid UTF-8 encoded NULL-terminated strings. Otherwise, behaviour is undefined.
  • with_binary_file requires the interaction contents to be valid for reads of size bytes, and it must be properly aligned and consecutive.

Based on the discussion in this Slack thread it would appear that with_binary_file can only be used with V3 (or later?) interactions.

This leaves unfortunately no way to specify an interaction with an arbitrary binary body for interactions using versions prior to V3 (at least, according to the documentation).

Proposed changes

Alternative 1

A new with_binary_body method

To avoid changing the behaviour of existing functions, the simplest change might just be to add a new with_binary_body function for V1 interactions. It would act in a similar way to with_body, albeit with little/no smarts in parsing the content.

Update with_binary_file and its documentation

The with_binary_file documentation should be updated to make it very clear that this function should not be used for interaction versions prior to V3.

Furthermore, I would suggest that the method be updated to do this check programmatically, and return a failure if with_binary_file is called with an incompatible version.

Alternative 2

Update with_binary_file behaviour based on interaction version

Instead of adding a new function, and having with_binary_file work only for interactions using v3 or later, the behaviour of with_binary_file could be adjusted based on the interaction version such that:

  • For versions prior to V3, with_binary_file does not perform intelligent mime-type detections.
  • For versions V3 or later, with_binary_file keeps its existing behaviour.
@JP-Ellis
Copy link
Contributor Author

Tagging @rholshausen, @mefellows, @YOU54F and @tienvx for comments and feedback. This was discussed before, and I want to make sure the above captures the relevant part of this discussion.

@rholshausen
Copy link
Contributor

I think option 1 is the way to go. Leave with_binary_file as is for now.

@tienvx
Copy link
Contributor

tienvx commented Nov 10, 2023

About option 2, I created a PR #327 . I think it's not released yet, so you have to build it manually

@rholshausen rholshausen added the bug Indicates an unexpected problem or unintended behavior label Nov 13, 2023
@rholshausen
Copy link
Contributor

0.4.10 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants