Skip to content

Commit

Permalink
Merge pull request #1968 from ballerina-platform/fix-client-return
Browse files Browse the repository at this point in the history
Fix inconsistencies in response binding
  • Loading branch information
TharmiganK authored Apr 25, 2024
2 parents 8da69b6 + cc680b7 commit ea075fc
Show file tree
Hide file tree
Showing 18 changed files with 224 additions and 32 deletions.
2 changes: 1 addition & 1 deletion ballerina-tests/http-advanced-tests/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
2 changes: 1 addition & 1 deletion ballerina-tests/http-client-tests/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright (c) 2024 WSO2 LLC. (http://www.wso2.org).
//
// WSO2 LLC. licenses this file to you under the Apache License,
// Version 2.0 (the "License"); you may not use this file except
// in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
import ballerina/http;
import ballerina/mime;
import ballerina/test;

service /api on new http:Listener(resBindingAdvancedPort) {

resource function get 'string() returns string {
return "Hello, World!";
}

resource function get urlEncoded() returns http:Ok {
return {
mediaType: mime:APPLICATION_FORM_URLENCODED,
body: {"name": "John", "age": "23"}
};
}

resource function get 'json() returns json {
return {"name": "John", "age": "23"};
}

resource function get 'xml() returns xml {
return xml `<message>Hello, World!</message>`;
}

resource function get byteArray() returns byte[] {
return [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
}
}

final http:Client clientEP = check new (string `localhost:${resBindingAdvancedPort}/api`);

@test:Config {}
function testAnydataResBindingWithDifferentContentType() returns error? {
anydata response = check clientEP->/'string;
test:assertEquals(response, "Hello, World!");

response = check clientEP->/urlEncoded;
map<string> expected = {"name": "John", "age": "23"};
test:assertEquals(response, expected);

response = check clientEP->/'json;
test:assertEquals(response, expected);

response = check clientEP->/'xml;
test:assertEquals(response, xml `<message>Hello, World!</message>`);

response = check clientEP->/'byteArray;
test:assertEquals(response, [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
}

@test:Config {}
function testResponseWithAnydataResBinding() returns error? {
http:Response|anydata response = check clientEP->/'string;
if response is http:Response {
test:assertEquals(check response.getTextPayload(), "Hello, World!");
} else {
test:assertFail("Invalid response type");
}

response = check clientEP->/urlEncoded;
if response is http:Response {
test:assertEquals(check response.getTextPayload(), "name=John&age=23");
} else {
test:assertFail("Invalid response type");
}

response = check clientEP->/'json;
if response is http:Response {
test:assertEquals(check response.getJsonPayload(), {"name": "John", "age": "23"});
} else {
test:assertFail("Invalid response type");
}

response = check clientEP->/'xml;
if response is http:Response {
test:assertEquals(check response.getXmlPayload(), xml `<message>Hello, World!</message>`);
} else {
test:assertFail("Invalid response type");
}

response = check clientEP->/'byteArray;
if response is http:Response {
test:assertEquals(check response.getBinaryPayload(), [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
} else {
test:assertFail("Invalid response type");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,5 @@ const int passthroughHostTestPort2 = 9608;

const int statusCodeBindingPort1 = 9609;
const int statusCodeBindingPort2 = 9610;

const int resBindingAdvancedPort = 9611;
2 changes: 1 addition & 1 deletion ballerina-tests/http-dispatching-tests/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
2 changes: 1 addition & 1 deletion ballerina-tests/http-interceptor-tests/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
2 changes: 1 addition & 1 deletion ballerina-tests/http-misc-tests/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
2 changes: 1 addition & 1 deletion ballerina-tests/http-resiliency-tests/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
2 changes: 1 addition & 1 deletion ballerina-tests/http-security-tests/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
2 changes: 1 addition & 1 deletion ballerina-tests/http-service-tests/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
2 changes: 1 addition & 1 deletion ballerina-tests/http-test-common/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
2 changes: 1 addition & 1 deletion ballerina-tests/http2-tests/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
2 changes: 1 addition & 1 deletion ballerina/Dependencies.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

[ballerina]
dependencies-toml-version = "2"
distribution-version = "2201.9.0-20240405-165800-4b163f78"
distribution-version = "2201.9.0-20240419-152500-bd530dd2"

[[package]]
org = "ballerina"
Expand Down
6 changes: 5 additions & 1 deletion ballerina/http_client_endpoint.bal
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ isolated function createStatusCodeResponseBindingError(boolean generalError, int

isolated function processResponse(Response|ClientError response, TargetType targetType, boolean requireValidation)
returns Response|anydata|ClientError {
if targetType is typedesc<Response> || response is ClientError {
if response is ClientError || hasHttpResponseType(targetType) {
return response;
}
int statusCode = response.statusCode;
Expand Down Expand Up @@ -741,3 +741,7 @@ isolated function externProcessResponseNew(Response response, typedesc<StatusCod
'class: "io.ballerina.stdlib.http.api.nativeimpl.ExternResponseProcessor",
name: "processResponse"
} external;

isolated function hasHttpResponseType(typedesc targetTypeDesc) returns boolean = @java:Method {
'class: "io.ballerina.stdlib.http.api.service.signature.builder.AbstractPayloadBuilder"
} external;
26 changes: 13 additions & 13 deletions ballerina/http_client_payload_builder.bal
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ isolated function getBuilderFromType(Response response, TargetType targetType) r
isolated function xmlPayloadBuilder(Response response, TargetType targetType) returns xml|ClientError? {
if targetType is typedesc<xml> {
return response.getXmlPayload();
} else if typeIncludedInUnion(targetType, xmlType) {
} else if matchingType(targetType, xmlType) {
xml|ClientError payload = response.getXmlPayload();
return payload is NoContentError ? (typeIncludedInUnion(targetType, nilType) ? () : payload) : payload;
return payload is NoContentError ? (matchingType(targetType, nilType) ? () : payload) : payload;
} else {
return getCommonError(response, targetType);
}
Expand All @@ -84,17 +84,17 @@ isolated function xmlPayloadBuilder(Response response, TargetType targetType) re
isolated function textPayloadBuilder(Response response, TargetType targetType) returns string|byte[]|ClientError? {
if targetType is typedesc<string> {
return response.getTextPayload();
} else if typeIncludedInUnion(targetType, stringType) {
} else if matchingType(targetType, string) {
string|ClientError payload = response.getTextPayload();
return payload is NoContentError ? (typeIncludedInUnion(targetType, nilType) ? () : payload) : payload;
return payload is NoContentError ? (matchingType(targetType, nilType) ? () : payload) : payload;
} else if targetType is typedesc<byte[]> {
return response.getBinaryPayload();
} else if typeIncludedInUnion(targetType, byteArrType) {
} else if matchingType(targetType, byteArrType) {
string|ClientError payload = response.getTextPayload();
if payload is string {
return payload.toBytes();
} else if payload is NoContentError {
return typeIncludedInUnion(targetType, nilType) ? () : payload;
return matchingType(targetType, nilType) ? () : payload;
}
return payload;
} else {
Expand All @@ -106,15 +106,15 @@ isolated function formPayloadBuilder(Response response, TargetType targetType) r
if targetType is typedesc<map<string>> {
string payload = check response.getTextPayload();
return getFormDataMap(payload);
} else if typeIncludedInUnion(targetType, mapStringType) {
} else if matchingType(targetType, mapStringType) {
string|ClientError payload = response.getTextPayload();
return payload is NoContentError ? (typeIncludedInUnion(targetType, nilType) ? () : payload) :
return payload is NoContentError ? (matchingType(targetType, nilType) ? () : payload) :
getFormDataMap(check payload);
} else if targetType is typedesc<string> {
return response.getTextPayload();
} else if typeIncludedInUnion(targetType, stringType) {
} else if matchingType(targetType, stringType) {
string|ClientError payload = response.getTextPayload();
return payload is NoContentError ? (typeIncludedInUnion(targetType, nilType) ? () : payload) : payload;
return payload is NoContentError ? (matchingType(targetType, nilType) ? () : payload) : payload;
} else {
return getCommonError(response, targetType);
}
Expand All @@ -123,10 +123,10 @@ isolated function formPayloadBuilder(Response response, TargetType targetType) r
isolated function blobPayloadBuilder(Response response, TargetType targetType) returns byte[]|ClientError? {
if targetType is typedesc<byte[]> {
return response.getBinaryPayload();
} else if typeIncludedInUnion(targetType, byteArrType) {
} else if matchingType(targetType, byteArrType) {
byte[]|ClientError payload = response.getBinaryPayload();
if payload is byte[] && payload.length() == 0 {
return typeIncludedInUnion(targetType, nilType) ? () : payload;
return matchingType(targetType, nilType) ? () : payload;
}
return payload;
} else {
Expand Down Expand Up @@ -199,6 +199,6 @@ isolated function performDataValidation(anydata payload, typedesc<anydata> targe
return payload;
}

isolated function typeIncludedInUnion(typedesc unionType, any targetType) returns boolean = @java:Method {
isolated function matchingType(typedesc unionType, any targetType) returns boolean = @java:Method {
'class: "io.ballerina.stdlib.http.api.service.signature.builder.AbstractPayloadBuilder"
} external;
2 changes: 2 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
### Added

- [Add status code response binding support for the HTTP client](https://github.com/ballerina-platform/ballerina-library/issues/6100)
- [Add response binding support for types union with `http:Response`](https://github.com/ballerina-platform/ballerina-library/issues/6416)

### Fixed

- [Address CVE-2024-29025 netty's vulnerability](https://github.com/ballerina-platform/ballerina-library/issues/6242)
- [Fix interceptor pipeline getting exited when there is a `nil` return](https://github.com/ballerina-platform/ballerina-library/issues/6278)
- [Fix response binding error for `anydata` type](https://github.com/ballerina-platform/ballerina-library/issues/6414)

## [2.10.12] - 2024-03-21

Expand Down
10 changes: 10 additions & 0 deletions native/spotbugs-exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -167,4 +167,14 @@
<Method name="getInstance"/>
<Bug pattern="MS_EXPOSE_REP" />
</Match>
<Match>
<Class name="io.ballerina.stdlib.http.api.service.signature.builder.AbstractPayloadBuilder" />
<Method name="matchingTypeInternal"/>
<Bug pattern="BC_UNCONFIRMED_CAST" />
</Match>
<Match>
<Class name="io.ballerina.stdlib.http.api.service.signature.builder.AbstractPayloadBuilder" />
<Method name="hasHttpResponseTypeInternal"/>
<Bug pattern="BC_UNCONFIRMED_CAST" />
</Match>
</FindBugsFilter>
Loading

0 comments on commit ea075fc

Please sign in to comment.