Listener does not handle double slashes in the URL

[chamil321]

Description:
URL contains double slashes

GET //tasks HTTP/1.1
host: localhost:9090
user-agent: ballerina
connection: keep-alive

The tracelog shows double slashes as expected. Check the serviceUrl trailing slash and the resourcePath leading slash
This leads to the String index out of range: -1.
Earlier HTTP client had this sanitization where we replaced mulitple slashes in to a single one and sent out the request. But later on it was removed it as it's kinda of modification done internally to the user input.

However the response should be 404, not 500 from the backend.

Steps to reproduce:

Client.bal

import ballerina/http;

public isolated client class Client {
    final http:Client clientEp;
    # Gets invoked to initialize the `connector`.
    #
    # + clientConfig - The configurations to be used when initializing the `connector` 
    # + serviceUrl - URL of the target service 
    # + return - An error if connector initialization failed 
    public isolated function init(http:ClientConfiguration clientConfig =  {}, string serviceUrl = "http://localhost:9090/") returns error? {
        http:Client httpEp = check new (serviceUrl, clientConfig);
        self.clientEp = httpEp;
        return;
    }
    #
    # + return - Ok 
    resource isolated function get tasks(string? groupId = ()) returns TaskResponse[]|error {
        string resourcePath = string `/tasks`;
        map<anydata> queryParam = {"groupId": groupId};
        resourcePath = resourcePath + check getPathForQueryParam(queryParam);
        TaskResponse[] response = check self.clientEp->get(resourcePath);
        return response;
    }
    #
    # + return - Ok 
    resource isolated function post tasks(TaskRequest payload) returns TaskResponse|error {
        string resourcePath = string `/tasks`;
        http:Request request = new;
        json jsonBody = check payload.cloneWithType(json);
        request.setPayload(jsonBody, "application/json");
        TaskResponse response = check self.clientEp->post(resourcePath, request);
        return response;
    }
    #
    # + return - Ok 
    resource isolated function get tasks/[string taskId]() returns TaskResponse|error {
        string resourcePath = string `/tasks/${getEncodedUri(taskId)}`;
        TaskResponse response = check self.clientEp->get(resourcePath);
        return response;
    }
    #
    # + return - Ok 
    resource isolated function put tasks/[string taskId](TaskRequest payload) returns TaskResponse|error {
        string resourcePath = string `/tasks/${getEncodedUri(taskId)}`;
        http:Request request = new;
        json jsonBody = check payload.cloneWithType(json);
        request.setPayload(jsonBody, "application/json");
        TaskResponse response = check self.clientEp->put(resourcePath, request);
        return response;
    }
}

Main.bal

import ballerina/io;

public type TaskResponseArr TaskResponse[];

public type TaskRequest record {
    string title;
    int groupId?;
    string status?;
};

public type TaskResponse record {
    int id;
    string title;
    int groupId;
    string status;
    string createdAt;
    string updatedAt;
};


public function main() returns error? {


    Client taskClient = check new();
    TaskResponse[] tasks = check taskClient->/tasks;
    io:println(tasks);


    // http:Client taskClient = check new ("http://localhost:9090");

    // json tasks = check taskClient->/tasks;
    // io:println(tasks);
}

Backend.bal

import ballerina/http;


public type TaskResponse record {
    int id;
    string title;
    int groupId;
    string status;
    string createdAt;
    string updatedAt;
};


# A service representing a network-accessible API
# bound to port `9090`.
service / on new http:Listener(9090) {

    # A resource for generating greetings
    # + name - the input string name
    # + return - string name with hello message or error
    resource function post tasks(string? groupId) returns TaskResponse[]|error {
        // Send a response back to the caller.
        TaskResponse[] arr = [{id:1, title: "ballerina", groupId: 0, status: "active", createdAt: "12.05", updatedAt: "2.30"},
        {id:2, title: "wso2", groupId: 0, status: "active", createdAt: "12.05", updatedAt: "2.30"}];
        return arr;
    }
}

Client output

error: Internal Server Error {"statusCode":500,"headers":{"content-type":["text/plain"],"date":["Fri, 21 Oct 2022 09:21:35 +0530"],"server":["ballerina"],"x-http2-stream-id":["1"]},"body":"String index out of range: -1"}

[TharmiganK]

Tested the behavior with the following service :

import ballerina/http;

service on new http:Listener(9090) {
    
    resource function get foo() returns http:Ok {
        return {body: "Greetings!"};
    }
}

cURL commands	Response
localhost:9090//foo	500 Internal Server Error with String index out of range: -1 message
localhost:9090//foo/bar	404 Not found
localhost:9090/foo//bar	404 Not found
localhost:9090///foo	200 OK

The above behavior is observed due to the parsing done by the URI library. And the following comment is added on the parser method :

// [//authority]<path>[?<query>]
//
// DEVIATION from RFC2396: We allow an empty authority component as
// long as it's followed by a non-empty path, query component, or
// fragment component.  This is so that URIs such as "file:///foo/bar"
// will parse.  This seems to be the intent of RFC2396, though the
// grammar does not permit it.  If the authority is empty then the
// userInfo, host, and port components are undefined.
//
// DEVIATION from RFC2396: We allow empty relative paths.  This seems
// to be the intent of RFC2396, but the grammar does not permit it.
// The primary consequence of this deviation is that "#f" parses as a
// relative URI with an empty path.

[chamil321]

Tested the behavior with the following service :

import ballerina/http;

service on new http:Listener(9090) {
    
    resource function get foo() returns http:Ok {
        return {body: "Greetings!"};
    }
}

cURL commands Response
localhost:9090//foo 500 Internal Server Error with String index out of range: -1 message
localhost:9090//foo/bar 404 Not found
localhost:9090/foo//bar 404 Not found
localhost:9090///foo 200 OK
The above behavior is observed due to the parsing done by the URI library. And the following comment is added on the parser method :

// [//authority]<path>[?<query>]
//
// DEVIATION from RFC2396: We allow an empty authority component as
// long as it's followed by a non-empty path, query component, or
// fragment component.  This is so that URIs such as "file:///foo/bar"
// will parse.  This seems to be the intent of RFC2396, though the
// grammar does not permit it.  If the authority is empty then the
// userInfo, host, and port components are undefined.
//
// DEVIATION from RFC2396: We allow empty relative paths.  This seems
// to be the intent of RFC2396, but the grammar does not permit it.
// The primary consequence of this deviation is that "#f" parses as a
// relative URI with an empty path.

Since the path component being passed(//foo) of the localhost:9090//foo to the URI create function, the // has been considered as the URI scheme operator.

[github-actions]

This issue is NOT closed with a proper Reason/ label. Make sure to add proper reason label before closing. Please add or leave a comment with the proper reason label now.

      - Reason/EngineeringMistake - The issue occurred due to a mistake made in the past.
      - Reason/Regression - The issue has introduced a regression.
      - Reason/MultipleComponentInteraction - Issue occured due to interactions in multiple components.
      - Reason/Complex - Issue occurred due to complex scenario.
      - Reason/Invalid - Issue is invalid.
      - Reason/Other - None of the above cases.