umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.4k stars 2.66k forks source link

Content Delivery API - /umbraco/delivery/api/v1/content/item/{path} not retrieving content #14298

Closed ZebSadiq closed 1 year ago

ZebSadiq commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

Umbraco version 12.0.0-rc2

Bug summary

When trying to retrieve content from the API using a relative path such as '/' always returns a 404 (Not found)

{ "type": "https://tools.ietf.org/html/rfc7231#section-6.5.4", "title": "Not Found", "status": 404, "traceId": "00-c7903a42b93e4d5f0df6a73d844d2079-00fece9be3a4f25f-00" }

image

image image

I have tried this with an upgraded project and a new v12 project.

Specifics

Request url: https://localhost:44320/umbraco/delivery/api/v1/content/item/%2F

it only works if you put the name of the page without the slashes e.g. 'aboutus'. However, this would not work for pages lower down in the content hierarchy.

Steps to reproduce

Create a new Umbraco project from here: https://psw.codeshare.co.uk/?InstallUmbracoTemplate=true&UmbracoTemplateVersion=12.0.0-rc2&Packages=&UserEmail=admin%40example.com&ProjectName=MyProject&CreateSolutionFile=true&SolutionName=MySolution&UseUnattendedInstall=true&DatabaseType=SQLite&UserPassword=1234567890&UserFriendlyName=Administrator&IncludeStarterKit=true&StarterKitPackage=clean&OnelinerOutput=false

Go to Settings > Examine Management > DeliveryApiContentIndex > Rebuild Index

Browse to /umbraco/swagger Select Umbraco Delivery API from the definition dropdown. Expand /umbraco/delivery/api/v1/content/item/{path} and click 'Try it out' In the path textbox, type in '/' Click 'Execute'. Notice the Responses:

image

Expected result / actual result

I would expect that my homepage node would be returned.

github-actions[bot] commented 1 year ago

Hi there @ZebSadiq!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

kjac commented 1 year ago

Hi @ZebSadiq,

Thank you for getting in touch - and thank you for trying out the RC 👏

When you want to fetch the root, you don't have to supply any path; anything after item in /umbraco/delivery/api/v1/content/item denotes the path you want. That is:

...etc etc.

We have noticed that Swagger has a tendency to URL encode the "/" character. This causes the request to me made for URL encoded paths, which do not identify an actually routable content item (REST resource). We're going to look into fixing Swagger ASAP. If you experience a request with "%2F" in it (like /umbraco/delivery/api/v1/content/item%2Fabout%2Fcontact), Swagger is misbehaving.

Could you please try again without any argument? And if you see "%2F" in Swagger requests, try performing them directly in the browser or using something like Postman?

ZebSadiq commented 1 year ago

Hi @kjac

I can confirm that the API returns the correct data when the '/' characters are not encoded. I tested this through the browser.

Thanks for the detailed explanation.

kjac commented 1 year ago

Thank you @ZebSadiq 😄

Swagger is not fond of "path based" endpoints (read: endpoint parameters that look like additional URL segments). While they are supported in the OpenAPI 3.0 specification, the general general consensus when it comes to Swagger UI seems to be that "new URL segment = new endpoint" ... which strictly speaking makes sense when one considers the nature of a standard REST API.

In our case, however, we need to map all URL segments below "/item" to the "/item" endpoint, and that's a struggle at the moment. But we'll look into it - the issue is in our backlog.

I'll keep this issue open until we have an update on the Swagger challenges.

kjac commented 1 year ago

Turns out OpenAPI does not support using special chars in path parameters, so we have to code our way around this. For the time being, ASP.NET Core acts up a little when it comes decoding path parameters with forward slashes in them, so we have to do it by hand until a fix arrives.

Fixed in https://github.com/umbraco/Umbraco-CMS/pull/14311

ZebSadiq commented 1 year ago

That's quite interesting. It's great that the path doesn't have to be encoded as it will result in less code and better performance.

kjac commented 1 year ago

It's a little silly, isn't it. I'm really a bit baffled why path parameters cannot be un-encoded in OpenAPI 3. I understand the reasoning from a strictly REST-ish perspective, but still... there must be plenty of use cases out there where a resource identifier contains forward slashes.

Oh well 🤷 at least we can code our way out of it. Now we'll be supporting both 😆

kjac commented 1 year ago

https://github.com/umbraco/Umbraco-CMS/pull/14311 has been merged now, so I'll go ahead and close this issue.

We are looking into a V12 RC3, which will contain this fix. It is still unknown when it will be out, we have a few more things we'd like in there.

Again, thank you so much for bringing this issue to our attention 👍