Skip to content

Conversation

Gusted
Copy link
Contributor

@Gusted Gusted commented Jan 28, 2022

  • Adding additional usernames which are already routes, it's safe to add them as any existing users should already have been "broken"(shouldn't be able to access profile etc.)

- Adding additional usernames which are already routes, it's safe to add
them as any existing users should already have been "broken"(shouldn't
be able to access profile etc.)
@Gusted Gusted added the type/enhancement An improvement of existing functionality label Jan 28, 2022
@Gusted Gusted added this to the 1.17.0 milestone Jan 28, 2022
@wxiaoguang
Copy link
Contributor

It's better to use the char ~ for all new paths in future, then we do not need to reserve anything more.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 28, 2022
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jan 28, 2022

Why serviceworker.js was reserved, it's under /assets/

(I mean the old code, since we are improving the code, maybe we could improve the legacy together)

@Gusted
Copy link
Contributor Author

Gusted commented Jan 28, 2022

Why serviceworker.js was reserved, it's under /assets/

Was introduced in #15834 #15834 (comment)

@wxiaoguang
Copy link
Contributor

Why serviceworker.js was reserved, it's under /assets/

Was introduced in #15834 #15834 (comment)

I can not get the point why should we keep it in the latest code base, maybe it's time to remove it?

And "less" (maybe more) could also be removed.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 28, 2022
@Gusted
Copy link
Contributor Author

Gusted commented Jan 28, 2022

I can not get the point why should we keep it in the latest code base, maybe it's time to remove it?

🤷🏽‍♀️ I'm not sure, better to leave it in then or silverwind gives the green light.

@lunny
Copy link
Member

lunny commented Jan 28, 2022

help and plugins should be kept for future usage.

@wxiaoguang
Copy link
Contributor

help and plugins should be kept for future usage.

What about using the char ~ for all new paths in future, then we do not need to reserve anything more.

For example: /~plugins/, no conflict anymore, no one would be hurt.

@lunny
Copy link
Member

lunny commented Jan 28, 2022

Or /-/ :)

@wxiaoguang
Copy link
Contributor

Yep, /-/ is also widely used in many systems.

@Gusted
Copy link
Contributor Author

Gusted commented Jan 28, 2022

So the removal of those "futured reserved names" is good?

@lunny
Copy link
Member

lunny commented Jan 28, 2022

So the removal of those "futured reserved names" is good?

After we have an agreement of /-/.

@wxiaoguang
Copy link
Contributor

From my experience, if we say about "agreement", most people just keep silence, a few people just disagree for no reason explained, then many useful ideas cannot be made to an agreement.

@Gusted
Copy link
Contributor Author

Gusted commented Jan 28, 2022

From my experience, if we say about "agreement", most people just keep silence, a few people just disagree for no reason explained, then many useful ideas cannot be made to an agreement.

Well, what about documenting a similar idea/process as Go has? https://github.com/golang/proposal#the-proposal-process whereby it's a "simple" agreed if nobody has a objection to it. Such that certain changes can be discussed and actually being worked on instead of being stalled because nobody wants to speak up.

@wxiaoguang
Copy link
Contributor

Most communities have similar processes. besides Golang, there are Python PEP https://www.python.org/dev/peps/#meta-peps-peps-about-peps-or-processes , PHP RFC https://wiki.php.net/rfc , etc.

If Gitea can have such documents, it would help, however, such documents need an agreement again. I hope we can get right things done and move on.

@wxiaoguang
Copy link
Contributor

Well, some comments are a liitle off-topic. Let back to this PR.

I think @lunny @Gusted and I all agree we can use /-/ in future and do not reserve any name, and remove the unused reserved names. Am I right?

And @go-gitea/maintainers , does any one have objection and new suggestions?

@KN4CK3R
Copy link
Member

KN4CK3R commented Jan 29, 2022

For the Docker/Open Container api I will need /v2/ (https://docs.docker.com/registry/spec/api/) and you can't change that to something else (like /<user>/v2).

@zeripath
Copy link
Contributor

not even /api/v2?

@KN4CK3R
Copy link
Member

KN4CK3R commented Jan 29, 2022

The last time I checked this I did not find a way. Here is the Gitlab change for example: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/10422 But I will test it again tomorrow.

@KN4CK3R
Copy link
Member

KN4CK3R commented Jan 31, 2022

Tested it again. The official Docker server has support for a different endpoint but the Docker CLI does not support it.
The spec does not allow a prefix too: https://github.com/opencontainers/distribution-spec/blob/v1.0.0/spec.md#api

Compose file:

version: '3'

services:
  docker-registry:
    image: registry:2
    environment:
      - REGISTRY_HTTP_PREFIX=/test/sub/

Opening <domain>/test/sub/v2 in a browser yields the expected JSON response.

docker tag ubuntu:latest <domain>/test/sub/ubuntu:latest
docker push <domain>/test/sub/ubuntu:latest

results in

"GET /v2/ HTTP/1.1" 404 19 "" "docker/20.10.12 go/go1.16.12 git-commit/459d0df kernel/5.10.60.1-microsoft-standard-WSL2 os/linux arch/amd64 UpstreamClient(Docker-Client/20.10.12 \\(windows\\))"
"HEAD /v2/test/sub/ubuntu/blobs/sha256:f3ef4ff62e0da0ef761ec1c8a578f3035bef51043e53ae1b13a20b3e03726d17 HTTP/1.1" 404 19 "" "docker/20.10.12 go/go1.16.12 git-commit/459d0df kernel/5.10.60.1-microsoft-standard-WSL2 os/linux arch/amd64 UpstreamClient(Docker-Client/20.10.12 \\(windows\\))"
"POST /v2/test/sub/ubuntu/blobs/uploads/ HTTP/1.1" 404 19 "" "docker/20.10.12 go/go1.16.12 git-commit/459d0df kernel/5.10.60.1-microsoft-standard-WSL2 os/linux arch/amd64 UpstreamClient(Docker-Client/20.10.12 \\(windows\\))"

The CLI does not know about the prefix and prepends the v2.

References:
kubernetes/ingress-nginx#4027
https://stackoverflow.com/questions/65888391/docker-registry-with-prefix-has-problem-with-docker-login
docker/cli#3255

@lunny
Copy link
Member

lunny commented Jan 31, 2022

Tested it again. The official Docker server has support for a different endpoint but the Docker CLI does not support it. The spec does not allow a prefix too: https://github.com/opencontainers/distribution-spec/blob/v1.0.0/spec.md#api

Compose file:

version: '3'

services:
  docker-registry:
    image: registry:2
    environment:
      - REGISTRY_HTTP_PREFIX=/test/sub/

Opening <domain>/test/sub/v2 in a browser yields the expected JSON response.

docker tag ubuntu:latest <domain>/test/sub/ubuntu:latest
docker push <domain>/test/sub/ubuntu:latest

results in

"GET /v2/ HTTP/1.1" 404 19 "" "docker/20.10.12 go/go1.16.12 git-commit/459d0df kernel/5.10.60.1-microsoft-standard-WSL2 os/linux arch/amd64 UpstreamClient(Docker-Client/20.10.12 \\(windows\\))"
"HEAD /v2/test/sub/ubuntu/blobs/sha256:f3ef4ff62e0da0ef761ec1c8a578f3035bef51043e53ae1b13a20b3e03726d17 HTTP/1.1" 404 19 "" "docker/20.10.12 go/go1.16.12 git-commit/459d0df kernel/5.10.60.1-microsoft-standard-WSL2 os/linux arch/amd64 UpstreamClient(Docker-Client/20.10.12 \\(windows\\))"
"POST /v2/test/sub/ubuntu/blobs/uploads/ HTTP/1.1" 404 19 "" "docker/20.10.12 go/go1.16.12 git-commit/459d0df kernel/5.10.60.1-microsoft-standard-WSL2 os/linux arch/amd64 UpstreamClient(Docker-Client/20.10.12 \\(windows\\))"

The CLI does not know about the prefix and prepends the v2.

References: kubernetes/ingress-nginx#4027 https://stackoverflow.com/questions/65888391/docker-registry-with-prefix-has-problem-with-docker-login docker/cli#3255

So we need to add v2 as reserved name.

@fnetX
Copy link
Contributor

fnetX commented Mar 4, 2022

Regarding "-", I think it's fine for all technical routes (like assets, avatars etc), but for everything visible to the end user, I personally prefer fancy URLs (/user, /settings, /admin etc). They are more intuitive to type and share (I often type URLs myself, and would probably forget about the dash easily).

@Gusted
Copy link
Contributor Author

Gusted commented Mar 4, 2022

Regarding "-", I think it's fine for all technical routes (like assets, avatars etc), but for everything visible to the end user, I personally prefer fancy URLs (/user, /settings, /admin etc). They are more intuitive to type and share (I often type URLs myself, and would probably forget about the dash easily).

The - would probably only be used for new routes(which currently is none in the planning IIRC)

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 27, 2022
@lunny lunny added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Mar 28, 2022
@lunny
Copy link
Member

lunny commented Mar 28, 2022

OK. Some instances may want to customize their reserved names list. So we may could also add a new list in ini configuration.

@Gusted
Copy link
Contributor Author

Gusted commented Mar 28, 2022

OK. Some instances may want to customize their reserved names list. So we may could also add a new list in ini configuration.

Should that be added within this PR? Seems like a good idea.

@lunny
Copy link
Member

lunny commented Mar 28, 2022

OK. Some instances may want to customize their reserved names list. So we may could also add a new list in ini configuration.

Should that be added within this PR? Seems like a good idea.

A standalone PR maybe reviewed faster.

@wxiaoguang wxiaoguang merged commit 43332a4 into go-gitea:main Mar 31, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 31, 2022
* giteaofficial/main:
  Update reserved usernames list (go-gitea#18438)
  Configure OpenSSH log level via Environment in Docker (go-gitea#19274)
  Use a more general (and faster) method to sanitize URLs with credentials (go-gitea#19239)
  [skip ci] Updated translations via Crowdin
  fix link to package registry docs (go-gitea#19268)
  Add Redis Sentinel Authentication Support (go-gitea#19213)
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
@Gusted Gusted deleted the update-reserved-usernames branch July 30, 2022 19:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants