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

[typescript-angular2] Bug: Request Content-Type other than json not allowed #6588

Closed
macjohnny opened this issue Sep 28, 2017 · 9 comments
Closed

Comments

@macjohnny
Copy link
Contributor

macjohnny commented Sep 28, 2017

Description

The changes in #6454 introduced setting the request content-type from the consumes property.

if (consumes != null && consumes.length > 0) {
    headers.set('Content-Type', consumes.filter(item => this.isJsonMime(item)).join(";"));
}

However, the content type is limited to json-like mime types. Therefore the content-type is set to an empty string if we e.g. upload a file and have

let consumes: string[] = [
     'multipart/form-data'
];

and since multipart/form-data is not a json mime-type, this results in setting and empty string value.
This in turn, results in an invalid content-type header being set, as explained e.g. here: angular/angular#11819

Content-Length: 315
Content-Type: , multipart/form-data; boundary=----WebKitFormBoundarys7BwSTvRA7Nf3WwQ
Host: petstore.swagger.io

This breaks e.g. the PetStore.uploadFile() API in the petStore example:

public uploadFile(petId: number, additionalMetadata?: string, file?: Blob, extraHttpRequestParams?: any): Observable<ApiResponse> {

Swagger-codegen version

2.3.0

Swagger declaration file content or url

See the petstore example

headers.set('Content-Type', consumes.filter(item => this.isJsonMime(item)).join(";"));

Command line used for generation
Steps to reproduce
Related issues/PRs

Maybe this is resolved with #6080

Suggest a fix/enhancement

Do not restrict the conten-type to json mime types.
Let angular detect the content-type automatically from the data.

@macjohnny
Copy link
Contributor Author

@stoetti @keradus @sebastianhaas could you have a look at this issue?
what do you think about letting angular determine the content-type and simply remove the following code:

if (consumes != null && consumes.length > 0) {
    headers.set('Content-Type', consumes.filter(item => this.isJsonMime(item)).join(";"));
}

@macjohnny
Copy link
Contributor Author

cc @wing328

@wing328
Copy link
Contributor

wing328 commented Sep 29, 2017

However, the content type is limited to json-like mime types. Therefore the content-type is set to an empty string if we e.g. upload a file and have

@macjohnny I think that's a bug that needs to be fix. It should not used only JSON-related MIME types.

The correct logic should be checking If JSON-related MIME is found in consumes (or produces), it should use that. Otherwise, just use the first one in the list if there's more than 1 defined.

@macjohnny
Copy link
Contributor Author

@wing328 i will file a PR as soon as #6574 is merged

@macjohnny
Copy link
Contributor Author

this issue would be resolved by merging #6295

@macjohnny
Copy link
Contributor Author

this issue was resolved with #6295

@wing328
Copy link
Contributor

wing328 commented Oct 3, 2017

@macjohnny are you going to file another PR to fix the following as originally reported in this issue?

if (consumes != null && consumes.length > 0) {
    headers.set('Content-Type', consumes.filter(item => this.isJsonMime(item)).join(";"));
}

I don't think isJsonMime is used at the moment:

grep -R isJsonMime modules/swagger-codegen/src/main/resources/typescript-angular/*
modules/swagger-codegen/src/main/resources/typescript-angular/api.service.mustache:    public isJsonMime(mime: string): boolean {

@macjohnny
Copy link
Contributor Author

macjohnny commented Oct 3, 2017

@wing328 the issue was resolved with the changes introduced in #6295. This is why in #6574 I removed the isJsonMime() method. See also the comment by @bedag-moo #6295 (comment)

@macjohnny
Copy link
Contributor Author

@wing328 btw are there already a release schedule for 2.3.0?

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

No branches or pull requests

2 participants