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

v2/package endpoint does not match specification #106

Closed
tomzo opened this issue Oct 1, 2018 · 8 comments
Closed

v2/package endpoint does not match specification #106

tomzo opened this issue Oct 1, 2018 · 8 comments

Comments

@tomzo
Copy link
Contributor

tomzo commented Oct 1, 2018

Do you want to request a feature or report a bug?

Bug / question

What did you do?

I have started porting e2e tests from nuget.org. One of first things that fails is upload into v2/package endpoint. I am a bit confused since in 3.0 specification it says that upload should be a multipart form. But actual nuget.org tests just stream the nupkg directly to endpoint.
@loic-sharma what do think, should BaGet accept multipart/form-data or application/octet-stream at v2/package ?

Currently when client from e2e tests is uploading, on server side we get null package and 400:

public async Task Upload(IFormFile package)
        {
            if (package == null)
            {
                HttpContext.Response.StatusCode = 400;
                return;
            }
@loic-sharma
Copy link
Owner

loic-sharma commented Oct 2, 2018

Great find! This seems like a bug on nuget.org or documentation that should be cleaned up! My gut feeling is that we should stick to the specification. Or maybe we could fallback to reading the request's body if package is null? What do you think?

EDIT: nuget.org uses HttpRequest.Files (see this code). According to the documentation, this is populated only when the HTTP request Content-Type value is "multipart/form-data".

@tomzo
Copy link
Contributor Author

tomzo commented Oct 2, 2018

Or maybe we could fallback to reading the request's body if package is null? What do you think?

Will do that. Same as nuget.org.

@loic-sharma
Copy link
Owner

I opened an issue on NuGet's documentation: https://github.com/NuGet/docs.microsoft.com-nuget/issues/1076

@BlythMeister
Copy link

I just started testing as found this exact issue when using paket to push to the V2 endpoint.

@tomzo
Copy link
Contributor Author

tomzo commented Oct 2, 2018

I got this working with paket now. I haven't opened a PR yet, but if you like to cherry-pick ai-traders@1826580

That's not the only thing for paket to work though. You'll need #104 too

@BlythMeister
Copy link

I'm ok to wait for now 😀
I'm just testing out tooling, but so far I like baget. 👍

@loic-sharma
Copy link
Owner

loic-sharma commented Nov 4, 2018

This should be fixed by 1f1ec07 now. Could you verify @BlythMeister?

@loic-sharma
Copy link
Owner

Closing this issue as it should now be fixed. Feel free to open a new issue if you run into any other problems!

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

No branches or pull requests

3 participants