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

Some fixes for AWS/S3 #54

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Some fixes for AWS/S3 #54

wants to merge 2 commits into from

Conversation

ochaton
Copy link

@ochaton ochaton commented May 12, 2023

I discovered that operations without empty description of operation.input does not work at all. I've added if into aws/init.lua and aws/request/build.lua

  • validate operation.input first before validate against it. s3/listBuckets does not accept any input.validator

s3_patch (aws/init.lua) seems to be not quite well written. When we change request.host we should change request.headers.Host as well, because of the signature. Also, operations such as listObjectsV2 and other meta operations requires non-empty path.

  • refix S3 fixes path: if path=="" we need to restore path="/"

The last part is not quite good :( It would be nice if we could parse xml output into Lua tables, we have operation.output descriptions for it. To do that we need to distinguish streaming content and API responses.

Streaming content (GetObject mostly, maybe GetObjectTorrent) it would be better to not read entire body into Lua memory, but provide body_reader from resty.http instead. Although I'm not sure that this behaviour should be fixed the way I did it

  • return raw body reader if operation output expects blob or streaming content

P.S. raw-api contains 17 "streaming":true lines, but necessarily all of the Outputs

	* validate operation.input first before validate against it
	s3/listBuckets does not accept any input.validator
	* refix S3 fixes path: if path=="" we need to restore path="/"
	* return raw body reader if operation output expects blob or
	  streaming content
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Vladislav Grubov seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@windmgc windmgc self-assigned this Aug 3, 2023
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