SwiftPackageIndex / SwiftPackageIndex-Server

The Swift Package Index is the place to find Swift packages!
https://swiftpackageindex.com
Apache License 2.0
543 stars 43 forks source link

Relative URLs in README incorrect format #844

Closed Sherlouk closed 3 years ago

Sherlouk commented 3 years ago

Images that rely on relative URLs are currently not working, at least not on Moya.

The relative URL it has detected (via data-readme-base-url) is "https:/raw.githubusercontent.com/Moya/Moya/master" which is not a valid URL due to the scheme only having one slash instead of two.

This means that the webpage tries to load https://staging.swiftpackageindex.com/raw.githubusercontent.com/Moya/Moya/master/web/diagram.png which is obviously not going to work.

https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/blob/main/Sources/App/Views/PackageController/PackageShow%2BModel.swift#L100 Interestingly we seem to use repository.readmeUrl - so the fact this is incorrect might be concerning?

Follow-on from #410

daveverwer commented 3 years ago

This is really strange as Moya was one of my test packages as I was developing the code to fix this issue. It also worked when first deployed, so this is a regression!

I think I have spotted what the problem is, the image is being replaced with an absolute version, as it should be, but there's only one slash in https:/raw.github rather than the two required https://raw.github.

Screenshot 2020-12-06 at 17 44 11@2x

I wonder if it's being escaped or something on the server as I'm sure those URLs are correct in the database. I just checked and this is Moya's URL in the db:

Screenshot 2020-12-06 at 17 46 26@2x

So, it's losing a slash somewhere between the database, and the front end.

Sherlouk commented 3 years ago

For what it's worth, it's not limited to Moya (I've checked the data tag for relative base URL on a number of packages and it's all showing the same!)

but yeah that's super peculiar

Sherlouk commented 3 years ago
    func testConverter() {
        XCTAssertEqual(NSString(string: "https://github.com/moya/moya/master/readme.md").deletingLastPathComponent, "https://github.com/moya/moya/master")
    }

XCTAssertEqual failed: ("https:/github.com/moya/moya/master") is not equal to ("https://github.com/moya/moya/master")

So it's this line: https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/blob/main/Sources/App/Views/PackageController/PackageShow%2BModel.swift#L100

Converting a URL using NSString doesn't seem to be good... 😬 @daveverwer

repository.readmeUrl.flatMap(URL.init(string:))?.deletingLastPathComponent().absoluteString seems to work better - but does give us a trailing slash so might need to handle that later in the chain. Currently struggling to get a test working to prove this though (trying to create a package which passes every guard)

Sherlouk commented 3 years ago

https://staging.swiftpackageindex.com/moya/moya

and we're back! 😅

finestructure commented 3 years ago

Beautiful :)

daveverwer commented 3 years ago

Thanks James!