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

Bug where route path component contains both parameter and constant #23

Open
ehyche opened this issue Feb 11, 2025 · 2 comments
Open

Bug where route path component contains both parameter and constant #23

ehyche opened this issue Feb 11, 2025 · 2 comments

Comments

@ehyche
Copy link

ehyche commented Feb 11, 2025

I am implementing a Swift Package Registry Service using swift-openapi-vapor. This service contains (among others) these two endpoints:

The OpenAPI spec for this service can be found here.

The bug is: the VaporTransport cannot distinguish between these two routes. When I hit the my server with the following URL:

http://127.0.0.1:8080/pointfreeco/swift-clocks/1.0.6.zip

then this gets parsed as a fetchReleaseMetadata route rather than a downloadSourceArchive route.

I believe the bug is here in VaporTransport.swift:

extension [Vapor.PathComponent] {
    init(_ path: String) {
        self = path.split(
            separator: "/",
            omittingEmptySubsequences: true
        ).map { parameter in
            if parameter.first == "{", parameter.last == "}" {
                return .parameter(String(parameter.dropFirst().dropLast()))
            } else {
                return .constant(String(parameter))
            }
        }
    }
}

Currently the OpenAPI path /{scope}/{name}/{version}.zip is getting parsed as:

[
    .parameter("scope"),
    .parameter("name"),
    .constant("{version}.zip")
]

I would assume that the OpenAPI path /{scope}/{name}/{version}.zip should get parsed as:

    .parameter("scope"),
    .parameter("name"),
    .parameter("version"),
    .constant(".zip"),

Is this a correct assumption?

If so, then I will put up a PR which accomplishes this.

@ehyche ehyche changed the title Bug where route path component parameter and constant Bug where route path component contains both parameter and constant Feb 11, 2025
@MahdiBM
Copy link
Collaborator

MahdiBM commented Feb 12, 2025

@0xTim WDYT? Admittedly I haven't had to add path-parameters where they're not surrounded between /s.
Can/Does Vapor support this?

@0xTim
Copy link
Member

0xTim commented Feb 12, 2025

So yes this is probably a bug, the issue is that Vapor's router doesn't currently support partially constant path parameters. So I suspect what you'll need to do is use the same handler and check in the handler if the path ends on .zip.

As an aside, having a fully dynamic path and a partially dynamic path is going to be ambiguous at best. We want to be able to support partially dynamic parameters in Vapor 5 but will need to consider the precedence of different path components because from a high level GET /{scope}/{name}/{version} would match just fine with /pointfreeco/swift-clocks/1.0.6.zip since there's no spec for 'Version'. So we will need to be careful not to tank performance in the router with whatever solution we come up with

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