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

The big fat modernization #65

Draft
wants to merge 38 commits into
base: main
Choose a base branch
from
Draft

The big fat modernization #65

wants to merge 38 commits into from

Conversation

lstrojny
Copy link
Contributor

No description provided.

@fcamp
Copy link

fcamp commented Sep 7, 2021

Hi @lstrojny,
I'd like to ask you if/when you plan to merge this and create a new tag

@DavidePastore
Copy link

Hi @lstrojny. Thanks for your commitment on this useful package. I'm trying to use this new branch but I think that I found a bug related to file upload. If you try to send a request with a file, you will get a 500 error from the Symfony server that is behind.
To reproduce this problem I used the following steps:

  • launch in debug mode an existing test in HttpMockPHPUnitIntegrationTest;
  • send a request with a file using another software (like Postman or curl);
  • check the response (500 status code and a body with the error).

What I see is the following:
image
It seems that the serialization process doesn't work well because it's not possible to serialize Symfony\Component\HttpFoundation\File\UploadedFile objects.

Is there a way to mitigate this issue? Thanks in advance

@lstrojny
Copy link
Contributor Author

@DavidePastore thank you very much for testing, much appreciated. Very good catch, that is a tricky one. I don’t have time to work on it right now but this is the direction you could look into:

  • On serialization, iterate over Request::$files and move all files to a temporary folder and store the relevant data of each files in the serialized request
  • Implement a subclass of UploadedFile like SerializableUploadedFile or something and instantiate that on deserialization and use the serialized data

@lstrojny
Copy link
Contributor Author

@fcamp I don’t have a schedule yet, sorry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants