We store the sizes as a hash from path to bytes like:
``` ruby
{'ci_artifacts.txt' => 27,
'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif' =>
71759,
'other_artifacts_0.1.2/doc_sample.txt' => 1314,
'rails_sample.jpg' => 35255,
'tests_encoding/utf8 test dir ✓/regular_file_2' => 7}
```
So that it's easier to access than reading gzip file again.
Fix an information disclosure when requesting access to a group containing private projects
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/19102.
The commit speaks for itself:
Fix an information disclosure when requesting access to a group containing private projects
The issue was with the `User#groups` and `User#projects` associations
which goes through the `User#group_members` and `User#project_members`.
Initially I chose to use a secure approach by storing the requester's
user ID in `Member#created_by_id` instead of `Member#user_id` because I
was aware that there was a security risk since I didn't know the
codebase well enough.
Then during the review, we decided to change that and directly store the
requester's user ID into `Member#user_id` (for the sake of simplifying
the code I believe), meaning that every `group_members` / `project_members`
association would include the requesters by default...
My bad for not checking that all the `group_members` / `project_members`
associations and the ones that go through them (e.g. `Group#users` and
`Project#users`) were made safe with the `where(requested_at: nil)` /
`where(members: { requested_at: nil })` scopes.
Now they are all secure.
See merge request !1973
Made the search bar on emoji menu sticky
## What does this MR do?
When scrolling down the emoji menu, the search bar disappears. For better UX, the search bar no stays at the top when scrolling.
## Screenshots (if relevant)
See merge request !4743
Image sizing
## What does this MR do?
Limits image height to fit the screen. The wrapping div is so the image is guaranteed to be a block element without the link area growing to be larger than the image itself.
## Are there points in the code the reviewer needs to double check?
Make sure this can't be done in a more performant or concise way with Banzai.
## Why was this MR needed?
Images were displayed at their full resolution, which made it difficult to read issues when the image height was greater than the viewport height (see #18861).
## What are the relevant issue numbers?
Fixes#18861.
## Screenshots (if relevant)
Before:
After:
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
cc: @jschatz1 @dzaporozhets @rspeicher
See merge request !4810
Test templates and GitLabCI parser againts each other
## What does this MR do?
Test the available templates against the preprocessor and vice versa
## Are there points in the code the reviewer needs to double check?
The dynamic creation of tests seems a little hacked. Is there a cleaner way?
## Does this MR meet the acceptance criteria?
- [ ] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added -- Seems unneeded
- [x] All builds are passing
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
See merge request !4898
Improve validations and error handling in new CI config entries
## What does this MR do?
This MR improves validation in new CI config.
## Why was this MR needed?
With that it will be easier to handle errors during validation and post-processing.
## What are the relevant issue numbers?
This is a continuation of #15060
See merge request !4560
Re-implement page-specific JS in a better way.
## What does this MR do?
This rebuilds the way we do page-specific JavaScript assets for a few reasons:
- The current implementation does not work with Subresource Integrity (!4808)
- The current implementation doesn't allow caching of libraries because each page we hook up with this system will have a separate application.js. Meaning that for every page that uses Ace Editor, we'd have to load Ace Editor plus any GitLab-specific scripts in the same file, making local caching of just Ace Editor impossible.
- The current implementation is rather hacky.
## Are there points in the code the reviewer needs to double check?
That Sprockets doesn't choke on this when we use precompiled assets.
## What are the relevant issue numbers?
#14372
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
cc: @jschatz1 @pavelloz
Thanks to @pavelloz for his example implementation which was very useful.
See merge request !4883
The issue was that `MergeRequest#mergeable?` returns false when the CI
state is not success and project.only_allow_merge_if_build_succeeds is
true. In this case `Projects::MergeRequestsController#merge` would
return the `:failed` status when enabling `merge_when_build_succeeds`,
thus leading to a weird state and the MR never automatically merged.
The fix is to disable the CI state check in the controller safeguard
that early return the `:failed` status.
Signed-off-by: Rémy Coutable <remy@rymai.me>
The issue was with the `User#groups` and `User#projects` associations
which goes through the `User#group_members` and `User#project_members`.
Initially I chose to use a secure approach by storing the requester's
user ID in `Member#created_by_id` instead of `Member#user_id` because I
was aware that there was a security risk since I didn't know the
codebase well enough.
Then during the review, we decided to change that and directly store the
requester's user ID into `Member#user_id` (for the sake of simplifying
the code I believe), meaning that every `group_members` / `project_members`
association would include the requesters by default...
My bad for not checking that all the `group_members` / `project_members`
associations and the ones that go through them (e.g. `Group#users` and
`Project#users`) were made safe with the `where(requested_at: nil)` /
`where(members: { requested_at: nil })` scopes.
Now they are all secure.
Signed-off-by: Rémy Coutable <remy@rymai.me>
This commit changes the way certain documents are rendered (currently
only Notes) and how documents are redacted. Previously both rendering
and redacting would run on a per document basis. The result of this was
that for every document we'd have to run countless queries just to
figure out if we could display a set of links or not.
This commit changes things around so that redacting Markdown documents
is no longer tied into the html-pipeline Gem. This in turn allows it to
redact multiple documents in a single pass, thus reducing the number of
queries needed.
In turn rendering issue/merge request notes has been adjusted to take
advantage of this new setup. Instead of rendering Markdown somewhere
deep down in a view the Markdown is rendered and redacted in the
controller (taking the current user and all that into account). This has
been done in such a way that the "markdown()" helper method can still be
used on its own.
This particular commit also paves the way for caching rendered HTML on
object level. Right now there's an accessor method Note#note_html which
is used for setting/getting the rendered HTML. Once we cache HTML on row
level we can simply change this field to be a column and call a "save"
whenever needed and we're pretty much done.
Fix settings_spec so that it can that run by itself
Running `spec/settings_spec.rb` would fail because it would attempt to load `1_settings.rb`, which would reference `Rails`. This constant was not loaded without `spec_helper.rb`.
See merge request !4893
Replace Haml with Hamlit
What this change does:
- It replaces HAML with [Hamlit](https://github.com/k0kubun/hamlit), a much more efficient implementation of HAML.
- It removes `haml-rails`.
- It adds `hamlit.rb` and removes `haml.rb`.
- It makes things faster and less memory-hungry!
See also #13201.
See merge request !3666
Fix pagination on sorts with lots of ties
## What does this MR do?
Fixes#18915. As we only order by the sorted column, we don't have any tie-breaker. Some orderings, like priority and weight, have lots of ties, so you can see duplicate results as you page through. (Timestamp columns are less susceptible to this.)
## Are there points in the code the reviewer needs to double check?
I just picked `id DESC`, this could as easily be `id ASC`.
## Why was this MR needed?
Postgres and MySQL don't guarantee that pagination with `LIMIT` and
`OFFSET` will work as expected if the ordering isn't unique. From the Postgres docs:
> When using `LIMIT`, it is important to use an `ORDER BY` clause that
> constrains the result rows into a unique order. Otherwise you will get
> an unpredictable subset of the query's rows
Before:
[1] pry(main)> issues = 1.upto(Issue.count).map { |i| Issue.sort('priority').page(i).per(1).map(&:id) }.flatten
[2] pry(main)> issues.count
=> 81
[3] pry(main)> issues.uniq.count
=> 42
After:
[1] pry(main)> issues = 1.upto(Issue.count).map { |i| Issue.sort('priority').page(i).per(1).map(&:id) }.flatten
[2] pry(main)> issues.count
=> 81
[3] pry(main)> issues.uniq.count
=> 81
See merge request !4878
Postgres and MySQL don't guarantee that pagination with `LIMIT` and
`OFFSET` will work if the ordering isn't unique. From the Postgres docs:
> When using `LIMIT`, it is important to use an `ORDER BY` clause that
> constrains the result rows into a unique order. Otherwise you will get
> an unpredictable subset of the query's rows
Before:
[1] pry(main)> issues = 1.upto(Issue.count).map { |i| Issue.sort('priority').page(i).per(1).map(&:id) }.flatten
[2] pry(main)> issues.count
=> 81
[3] pry(main)> issues.uniq.count
=> 42
After:
[1] pry(main)> issues = 1.upto(Issue.count).map { |i| Issue.sort('priority').page(i).per(1).map(&:id) }.flatten
[2] pry(main)> issues.count
=> 81
[3] pry(main)> issues.uniq.count
=> 81
Fix subsequent SAML sign ins
Fixes a bug when `auto_link_ldap_user` is `true` that causes SAML users to be unable to sign in a second time.
Fix the problem for https://gitlab.zendesk.com/agent/tickets/22546
See merge request !4718
Handle external issues in IssueReferenceFilter
Handling of external issues was broken when I refactored
`IssueReferenceFilter` to use fewer SQL queries.
Fixes#18827
See merge request !4789
Display group/project access requesters separately in admin
## What does this MR do?
It displays the access requesters in a separate list in group & project members pages.
It also harmonize the members counter UI to use `%span.badge` everywhere (in the admin & non-admin members views).
## Are there points in the code the reviewer needs to double check?
No.
## Why was this MR needed?
To not confuse access requesters with actual members.
## What are the relevant issue numbers?
Closes#18871.
## Screenshots
### Group members
| Before | After |
| --------- | ---- |
|  |  |
### Project members
| Before | After |
| --------- | ---- |
|  |  |
### Admin group members
| Before | After |
| --------- | ---- |
|  |  |
### Admin project members
| Before | After |
| --------- | ---- |
|  |  |
## Does this MR meet the acceptance criteria?
- [x] No CHANGELOG since this is related to the original "request access" MR.
- [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !4798
Lin Jen-Shin
Robert Speicher
Drew Blessing
Douwe Maan
Robert Speicher
Ruben Davila
Z.J. van de Weg
Jacob Schatz
Rémy Coutable
Connor Shea
Stan Hu
Simon Welsh
Yorick Peterse
Alfredo Sumaran
Marc Siegfriedt
Kamil Trzcinski
Sean McGivern
Paco Guzman
James Lopez
Douglas Barbosa Alexandre