User input content length / size limits #8570

Closed
opened 2025-11-02 08:11:06 -06:00 by GiteaMirror · 13 comments
Owner

Originally created by @fnetX on GitHub (Feb 16, 2022).

Feature Description

Recently, in #16765, Gitea migrated the issue comment size to allow 4GB text size, an easy way to shoot of every Gitea instance (small on Raspberry - does it even fit in RAM?, and large ones like Codeberg, too).

In many places, Gitea does not do any size checking and just relies on the database to handle stuff (it either throws a 500 error if data is too long, or if SQL mode is on truncate it just truncates data).

It would be great, if could go through / create a list of user input fields and either choose sensible defaults or make it configurable, improve UI feedback (ideally before submission).

Related issues that are kind-of included here:

  • Add a character count or visible warning to repo descriptions #14251
  • ... I know there are more but can't find them, I'll search more carefully later

Screenshots

No response

Originally created by @fnetX on GitHub (Feb 16, 2022). ### Feature Description Recently, in #16765, Gitea migrated the issue comment size to allow 4GB text size, an easy way to shoot of every Gitea instance (small on Raspberry - does it even fit in RAM?, and large ones like Codeberg, too). In many places, Gitea does not do any size checking and just relies on the database to handle stuff (it either throws a 500 error if data is too long, or if SQL mode is on truncate it just truncates data). It would be great, if could go through / create a list of user input fields and either choose sensible defaults or make it configurable, improve UI feedback (ideally before submission). Related issues that are kind-of included here: - Add a character count or visible warning to repo descriptions #14251 - ... I know there are more but can't find them, I'll search more carefully later ### Screenshots _No response_
GiteaMirror added the issue/confirmedtype/summary labels 2025-11-02 08:11:06 -06:00
Author
Owner

@lunny commented on GitHub (Feb 16, 2022):

Every input form has a struct in services/forms which has some limitations. You can just send PR to change them.

@lunny commented on GitHub (Feb 16, 2022): Every input form has a struct in `services/forms` which has some limitations. You can just send PR to change them.
Author
Owner

@fnetX commented on GitHub (Feb 16, 2022):

Implementing hard limits does not prevent users in the UI. But yes, I'll check it out and see if I can do something. It will at least help us to prevent being shot with large issue comments.

@fnetX commented on GitHub (Feb 16, 2022): Implementing hard limits does not prevent users in the UI. But yes, I'll check it out and see if I can do something. It will at least help us to prevent being shot with large issue comments.
Author
Owner

@fnetX commented on GitHub (Feb 16, 2022):

Do the form structs also work for the API? I'm basically having a summary issue here - we need both the limits and UI feedback rather than server errors and truncated data.

The limits should thus obviously apply for the API, too.

@fnetX commented on GitHub (Feb 16, 2022): Do the form structs also work for the API? I'm basically having a summary issue here - we need both the limits and UI feedback rather than server errors and truncated data. The limits should thus obviously apply for the API, too.
Author
Owner

@lunny commented on GitHub (Feb 16, 2022):

API also has API's form, it maybe different.

@lunny commented on GitHub (Feb 16, 2022): API also has API's form, it maybe different.
Author
Owner

@fnetX commented on GitHub (Feb 16, 2022):

It's notable that the recent PRs from @wxiaoguang that only set LONGTEXT for every thing kinda defeats the purpose of " Restrict some tables' columns size #14075 " which tries to limit some column sizes for various reasons. LONGTEXT sounds just wrong, especially with no limits. And if we set limits again, LONGTEXT would be unnecessarily too much. IMHO, this sounds unreasonable.

And IIRC, the database was once migrated from MEDIUMTEXT to TEXT, too.

@fnetX commented on GitHub (Feb 16, 2022): It's notable that the recent PRs from @wxiaoguang that only set LONGTEXT for every thing kinda defeats the purpose of " Restrict some tables' columns size #14075 " which tries to limit some column sizes for various reasons. LONGTEXT sounds just wrong, especially with no limits. And if we set limits again, LONGTEXT would be unnecessarily too much. IMHO, this sounds unreasonable. And IIRC, the database was once migrated from MEDIUMTEXT to TEXT, too.
Author
Owner

@wxiaoguang commented on GitHub (Feb 16, 2022):

It's notable that the recent PRs from @wxiaoguang that only set LONGTEXT for every thing kinda defeats the purpose of " Restrict some tables' columns size #14075 " which tries to limit some column sizes for various reasons. LONGTEXT sounds just wrong, especially with no limits. And if we set limits again, LONGTEXT would be unnecessarily too much. IMHO, this sounds unreasonable.

And IIRC, the database was once migrated from MEDIUMTEXT to TEXT, too.

No, it is not the case. All the databases except MySQL treat TEXT as nearly "no-limitation". So my PR only made these database behaviors consistent. I have made a comment in the PR https://github.com/go-gitea/gitea/pull/16765 clearly: And, no other database has such limitation as MySQL. So it's good to make issue system's behavior the same with different databases.

So, what I did is not wrong. You should keep in mind that you must use a correct mechanism to protect your server. For example, an attacker can post very large data to your server to do DoS. That's why nginx has an option called client_max_body_size. By no mean you should try to limit anything from database side to achieve security purpose.

@wxiaoguang commented on GitHub (Feb 16, 2022): > It's notable that the recent PRs from @wxiaoguang that only set LONGTEXT for every thing kinda defeats the purpose of " Restrict some tables' columns size #14075 " which tries to limit some column sizes for various reasons. LONGTEXT sounds just wrong, especially with no limits. And if we set limits again, LONGTEXT would be unnecessarily too much. IMHO, this sounds unreasonable. > > And IIRC, the database was once migrated from MEDIUMTEXT to TEXT, too. No, it is not the case. All the databases except MySQL treat TEXT as nearly "no-limitation". So my PR only made these database behaviors consistent. I have made a comment in the PR https://github.com/go-gitea/gitea/pull/16765 clearly: `And, no other database has such limitation as MySQL. So it's good to make issue system's behavior the same with different databases.` So, what I did is not wrong. You should keep in mind that you must use a correct mechanism to protect your server. For example, an attacker can post very large data to your server to do DoS. That's why nginx has an option called `client_max_body_size`. By no mean you should try to limit anything from database side to achieve security purpose.
Author
Owner

@fnetX commented on GitHub (Feb 17, 2022):

Yes, I know that the protection should not occur at database level (that's why we don't just modify the column again ;-)), but it should happen at the application level (in Gitea - thus the feature request). And also, you should consider what "no-limitation" means. Does it really need to store 4GB? Or would MEDIUMTEXT be enough, for example?

If we would limit the max body size via proxy, as you propose, we'd be no better than the error 500 Gitea shows when the mysql truncation mode is on. If we do limit, and I think this is important, there should be some feedback to the user, ideally before even submitting a form (but another check afterwards to give feedback to API clients, too).

Also, if you really require someone to fiddle around with the webserver config in order to protect Gitea, this is a topic for the docs. I bet there are many many Gitea instances (if not >99%) that don't use such means of "protection".

@fnetX commented on GitHub (Feb 17, 2022): Yes, I know that the protection should not occur at database level (that's why we don't just modify the column again ;-)), but it should happen at the application level (in Gitea - thus the feature request). And also, you should consider what "no-limitation" means. Does it really need to store 4GB? Or would MEDIUMTEXT be enough, for example? If we would limit the max body size via proxy, as you propose, we'd be no better than the error 500 Gitea shows when the mysql truncation mode is on. If we do limit, and I think this is important, there should be some feedback to the user, ideally before even submitting a form (but another check afterwards to give feedback to API clients, too). Also, if you really require someone to fiddle around with the webserver config in order to protect Gitea, this is a topic for the docs. I bet there are many many Gitea instances (if not >99%) that don't use such means of "protection".
Author
Owner

@wxiaoguang commented on GitHub (Feb 17, 2022):

Yes, I know that the protection should not occur at database level (that's why we don't just modify the column again ;-)), but it should happen at the application level (in Gitea - thus the feature request). And also, you should consider what "no-limitation" means. Does it really need to store 4GB? Or would MEDIUMTEXT be enough, for example?

If you think it is a problem, then there are a lot of new problems, for example, do you want to limit how many issues/comments can a user create? An attacker can create a lot of issues with MEDIUMTEXT to occupy the 4G storage too. So the LONGTEXT is not wrong, MEDIUMTEXT is not necessary and doesn't bring benefits.

If we would limit the max body size via proxy, as you propose, we'd be no better than the error 500 Gitea shows when the mysql truncation mode is on. If we do limit, and I think this is important, there should be some feedback to the user, ideally before even submitting a form (but another check afterwards to give feedback to API clients, too).

I have no objection to valid the input size by code, I was talking about that "LONGTEXT is not a problem" (which was considered to be a problem in the previous comment).

Also, if you really require someone to fiddle around with the webserver config in order to protect Gitea, this is a topic for the docs. I bet there are many many Gitea instances (if not >99%) that don't use such means of "protection".

It's a new topic, I just took it as an example to explain why column limitation on database side doesn't really help security.

@wxiaoguang commented on GitHub (Feb 17, 2022): > Yes, I know that the protection should not occur at database level (that's why we don't just modify the column again ;-)), but it should happen at the application level (in Gitea - thus the feature request). And also, you should consider what "no-limitation" means. Does it really need to store 4GB? Or would MEDIUMTEXT be enough, for example? If you think it is a problem, then there are a lot of new problems, for example, do you want to limit how many issues/comments can a user create? An attacker can create a lot of issues with MEDIUMTEXT to occupy the 4G storage too. So the LONGTEXT is not wrong, MEDIUMTEXT is not necessary and doesn't bring benefits. > If we would limit the max body size via proxy, as you propose, we'd be no better than the error 500 Gitea shows when the mysql truncation mode is on. If we do limit, and I think this is important, there should be some feedback to the user, ideally before even submitting a form (but another check afterwards to give feedback to API clients, too). I have no objection to valid the input size by code, I was talking about that "LONGTEXT is not a problem" (which was considered to be a problem in the previous comment). > Also, if you really require someone to fiddle around with the webserver config in order to protect Gitea, this is a topic for the docs. I bet there are many many Gitea instances (if not >99%) that don't use such means of "protection". It's a new topic, I just took it as an example to explain why column limitation on database side doesn't really help security.
Author
Owner

@stale[bot] commented on GitHub (Apr 19, 2022):

This issue has been automatically marked as stale because it has not had recent activity. I am here to help clear issues left open even if solved or waiting for more insight. This issue will be closed if no further activity occurs during the next 2 weeks. If the issue is still valid just add a comment to keep it alive. Thank you for your contributions.

@stale[bot] commented on GitHub (Apr 19, 2022): This issue has been automatically marked as stale because it has not had recent activity. I am here to help clear issues left open even if solved or waiting for more insight. This issue will be closed if no further activity occurs during the next 2 weeks. If the issue is still valid just add a comment to keep it alive. Thank you for your contributions.
Author
Owner

@fnetX commented on GitHub (Apr 19, 2022):

Still relevant IMO.

@fnetX commented on GitHub (Apr 19, 2022): Still relevant IMO.
Author
Owner

@wxiaoguang commented on GitHub (Mar 29, 2023):

I see this gets closed. Some of my thoughts:

It would be great, if could go through / create a list of user input fields and either choose sensible defaults or make it configurable, improve UI feedback (ideally before submission).

There are so many, at the moment I have no idea about how to "list" them or make them "configurable" (overcomplicated?).

We can always improve anything, but I have no idea about how to do that.

Still relevant IMO.

Could you elaborate about the details of the proposal? And if there could be a detailed feasible document or a PR demo, it could help.

@wxiaoguang commented on GitHub (Mar 29, 2023): I see this gets closed. Some of my thoughts: > It would be great, if could go through / create a list of user input fields and either choose sensible defaults or make it configurable, improve UI feedback (ideally before submission). There are so many, at the moment I have no idea about how to "list" them or make them "configurable" (overcomplicated?). We can always improve anything, but I have no idea about how to do that. > Still relevant IMO. Could you elaborate about the details of the proposal? And if there could be a detailed feasible document or a PR demo, it could help.
Author
Owner

@lunny commented on GitHub (Mar 29, 2023):

I think the issue is just like a summary for all input forms but not a concreate problem. Maybe we one by one. I list some of them, it's welcome to improve them.

  • Issue title and content length / size limitation
  • Comment content length / size limitation
  • Code editor filepath and content length / size limitation
  • Release title and content length/size limitation
  • Login name input box length/size limitation
  • Login password input box length/size limitation
@lunny commented on GitHub (Mar 29, 2023): I think the issue is just like a summary for all input forms but not a concreate problem. Maybe we one by one. I list some of them, it's welcome to improve them. - [ ] Issue title and content length / size limitation - [ ] Comment content length / size limitation - [ ] Code editor filepath and content length / size limitation - [ ] Release title and content length/size limitation - [ ] Login name input box length/size limitation - [ ] Login password input box length/size limitation
Author
Owner

@wxiaoguang commented on GitHub (Dec 29, 2023):

Since Codeberg is running a self-maintained fork, I don't see any end user really needs this. So I think it can be closed.

Again, #16765 doesn't bring any real or new problem.

@wxiaoguang commented on GitHub (Dec 29, 2023): Since Codeberg is running a self-maintained fork, I don't see any end user really needs this. So I think it can be closed. Again, #16765 doesn't bring any real or new problem.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#8570