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

All pages and API endpoints (including non-existent ones) return 200 #1947

Open
5 tasks done
lonix1 opened this issue Jul 7, 2023 · 3 comments
Open
5 tasks done

All pages and API endpoints (including non-existent ones) return 200 #1947

lonix1 opened this issue Jul 7, 2023 · 3 comments
Labels
bug Something isn't working server
Milestone

Comments

@lonix1
Copy link
Contributor

lonix1 commented Jul 7, 2023

Component

server,

Describe the bug

In trying to setup server healthchecks I uncovered an issue related to the API endpoints in version "next".

To eliminate weird config errors, differences in environments, etc., I am going to show a repro performed from inside the server container.

Set up curl:

$ docker exec -it woodpecker-server sh
$ apk add --no-cache curl

Request a non-existent page:

$ curl http://localhost:8000/foo
<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <link rel="alternate icon" type="image/png" href="/favicons/favicon-light-default.png" id="favicon-png" />
    <link rel="icon" type="image/svg+xml" href="/favicons/favicon-light-default.svg" id="favicon-svg" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <meta name="theme-color" content="#65a30d" />
    <title>Woodpecker</title>
    <script type="" src="/web-config.js"></script>
    <script type="module" crossorigin src="/assets/index-4b7b57db.js"></script>
    <link rel="stylesheet" href="/assets/index-f123c06a.css">
  </head>
  <body>
    <div id="app"></div>

  </body>
</html>

That is basically an "empty" page. Let's look at the response code:

$ curl -I http://localhost:8000/foo
HTTP/1.1 200 OK
X-Woodpecker-Version: next-45319b24
# ...etc.

That should have returned 404.

Now let's request a non-existent API endpoint:

$ curl -I http://localhost:8000/api/bar
HTTP/1.1 200 OK
X-Woodpecker-Version: next-45319b24
# ...etc.

Same thing.

Now let's request a real API endpoint, for healthchecks:

$ curl http://localhost:8000/api/healthz
<!DOCTYPE html>
<html lang="en">
  # ...
</html>

$ curl -I http://localhost:8000/api/healthz
HTTP/1.1 200 OK
X-Woodpecker-Version: next-45319b24
# ...etc.

Same thing. Also, it should not return a page, only a REST response.

Clean up:

$ apk delete curl
$ exit

Summary:

  • both existent and non-existent endpoints and pages respond with 200
  • that means the /api/healthz endpoint is not working: it's a "proof of life" rather than "proof of health"
  • and the health endpoint should only return a json REST response, not a page

System Info

version next-45319b24-alpine
dockerised

Additional context

No response

Validations

  • Read the Contributing Guidelines.
  • Read the docs.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • Checked that the bug isn't fixed in the next version already [https://woodpecker-ci.org/faq#which-version-of-woodpecker-should-i-use]
  • Check that this is a concrete bug. For Q&A join our Discord Chat Server or the Matrix room.
@lonix1 lonix1 added the bug Something isn't working label Jul 7, 2023
@lonix1 lonix1 mentioned this issue Jul 7, 2023
5 tasks
@lonix1 lonix1 changed the title All API endpoints and pages return 200 including non-existent ones All pages and API endpoints (including non-existent ones) return 200 Jul 7, 2023
@6543 6543 added the server label Jul 7, 2023
@6543 6543 added this to the 1.1.0 milestone Jul 7, 2023
@anbraten
Copy link
Member

Maybe its coming from the sub-path changes #1783 #1714

@qwerty287
Copy link
Contributor

No, this comes from the web UI. If a route is not found, the index.html of the web UI is served. Not found will be handled inside the UI then.

@qwerty287
Copy link
Contributor

It is weird though that this is also done for the healthcheck endpoint 🤔

@pat-s pat-s modified the milestones: 2.0.0, 2.x.x Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working server
Projects
None yet
Development

No branches or pull requests

5 participants