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

Change default size of issue/pr attachments and repo file #27946

Merged
merged 3 commits into from
Nov 13, 2023

Conversation

lng2020
Copy link
Member

@lng2020 lng2020 commented Nov 7, 2023

As title. Some attachments and file sizes can easily be larger than these limits

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 7, 2023
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 7, 2023
@silverwind
Copy link
Member

silverwind commented Nov 7, 2023

How about 50MiB which is also the default on GitHub currently? We do want to find some reasonable default values that prevent abuse from uploading too large files.

Edit: 50 MiB is limit for repo files, but I guess it will work just as well as a reasonable default for attachments etc.

@lng2020
Copy link
Member Author

lng2020 commented Nov 10, 2023

50MiB is okay for the repo file. But the attachment ->MAX_SIZE is also used in the release page. I think it's better to use a larger one. But we should probably separate the limit of PR/issue attachment and the release file.
image

@lng2020
Copy link
Member Author

lng2020 commented Nov 10, 2023

BTW, the alert of the file exceeding limits is broken. Haven't found out root reason yet but it's probably because the function initGlobalDropzone

export function initGlobalDropzone() {
// Dropzone
for (const el of document.querySelectorAll('.dropzone')) {
const $dropzone = $(el);
const _promise = createDropzone(el, {
url: $dropzone.data('upload-url'),
headers: {'X-Csrf-Token': csrfToken},
maxFiles: $dropzone.data('max-file'),
maxFilesize: $dropzone.data('max-size'),
acceptedFiles: (['*/*', ''].includes($dropzone.data('accepts'))) ? null : $dropzone.data('accepts'),
addRemoveLinks: true,
dictDefaultMessage: $dropzone.data('default-message'),
dictInvalidFileType: $dropzone.data('invalid-input-type'),
dictFileTooBig: $dropzone.data('file-too-big'),
dictRemoveFile: $dropzone.data('remove-file'),
timeout: 0,
thumbnailMethod: 'contain',
thumbnailWidth: 480,
thumbnailHeight: 480,
init() {
this.on('success', (file, data) => {
file.uuid = data.uuid;
const input = $(`<input id="${data.uuid}" name="files" type="hidden">`).val(data.uuid);
$dropzone.find('.files').append(input);
// Create a "Copy Link" element, to conveniently copy the image
// or file link as Markdown to the clipboard
const copyLinkElement = document.createElement('div');
copyLinkElement.className = 'gt-text-center';
// The a element has a hardcoded cursor: pointer because the default is overridden by .dropzone
copyLinkElement.innerHTML = `<a href="#" style="cursor: pointer;">${svg('octicon-copy', 14, 'copy link')} Copy link</a>`;
copyLinkElement.addEventListener('click', async (e) => {
e.preventDefault();
let fileMarkdown = `[${file.name}](/attachments/${file.uuid})`;
if (file.type.startsWith('image/')) {
fileMarkdown = `!${fileMarkdown}`;
} else if (file.type.startsWith('video/')) {
fileMarkdown = `<video src="/attachments/${file.uuid}" title="${htmlEscape(file.name)}" controls></video>`;
}
const success = await clippie(fileMarkdown);
showTemporaryTooltip(e.target, success ? i18n.copy_success : i18n.copy_error);
});
file.previewTemplate.append(copyLinkElement);
});
this.on('removedfile', (file) => {
$(`#${file.uuid}`).remove();
if ($dropzone.data('remove-url')) {
POST($dropzone.data('remove-url'), {
data: new URLSearchParams({file: file.uuid}),
});
}
});
},
});
}
}

Edited: fixed in #27985

@silverwind
Copy link
Member

silverwind commented Nov 10, 2023

Right, releases need a separate bigger limit. GitHub has this at 2GB per file currently.

@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 Nov 13, 2023
@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 Nov 13, 2023
@wxiaoguang wxiaoguang added the backport/v1.21 This PR should be backported to Gitea 1.21 label Nov 13, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 13, 2023
@lunny lunny enabled auto-merge (squash) November 13, 2023 14:18
@lunny lunny merged commit 0678c82 into go-gitea:main Nov 13, 2023
23 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Nov 13, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 13, 2023
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Nov 13, 2023
…7946)

As title. Some attachments and file sizes can easily be larger than
these limits
@GiteaBot GiteaBot added the backport/done All backports for this PR have been created label Nov 13, 2023
6543 pushed a commit that referenced this pull request Nov 13, 2023
…28017)

Backport #27946 by @lng2020

As title. Some attachments and file sizes can easily be larger than
these limits

Co-authored-by: Nanguan Lin <70063547+lng2020@users.noreply.github.com>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 14, 2023
* upstream/main:
  fixed duplicate attachments on dump on windows (go-gitea#28019)
  [skip ci] Updated translations via Crowdin
  packages: Calculate package size quota using package creator ID instead of owner ID (go-gitea#28007)
  Dont leak private users via extensions (go-gitea#28023)
  Improve profile for Organizations (go-gitea#27982)
  Enable system users search via the API (go-gitea#28013)
  Enable system users for comment.LoadPoster (go-gitea#28014)
  Change default size of issue/pr attachments and repo file (go-gitea#27946)
  Fix missing mail reply address (go-gitea#27997)
silverwind pushed a commit that referenced this pull request Nov 17, 2023
#27946 forgets to change them in
code. Sorry about that.
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Nov 17, 2023
silverwind pushed a commit that referenced this pull request Nov 17, 2023
Backport #28100 by @lng2020

#27946 forgets to change them in
code. Sorry about that.

Co-authored-by: Nanguan Lin <70063547+lng2020@users.noreply.github.com>
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this pull request Jan 17, 2024
…7946)

As title. Some attachments and file sizes can easily be larger than
these limits
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this pull request Jan 17, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/docs size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants