This is a backport of 84e13e9.
We caught it in logs, race condition led to this error:
ActiveRecord::RecordNotUnique
(PG::UniqueViolation: ERROR: duplicate key value violates unique constraint "user_statuses_pkey"
DETAIL: Key (user_id)=(15) already exists.)
The reason the problem happened was that we were checking if a user has status and if not inserting status:
if user_status
...
else
self.user_status = UserStatus.create!(status)
end
The problem is that it's possible that another request will insert status just after we check if status exists and just before our request call `UserStatus.create!(status)`. Using `upsert` fixes the problem because under the hood `upsert` generates the only SQL request that uses "INSERT ... ON CONFLICT DO UPDATE". So we do everything in one SQL query, and that query takes care of resolving possible conflicts.
Currently `Topic#pm_topic_count` is a count of all personal messages tagged for a given tag. As a result, any user with access to PM tags can poll a sensitive tag to determine if a new personal message has been created using that tag even if the user does not have access to the personal message. We classify this as a minor leak in sensitive information.
With this commit, `Topic#pm_topic_count` is hidden from users by default unless the `display_personal_messages_tag_counts` site setting is enabled.
The presence service would retry `/presence/update` requests every second (or immediately in tests) in case where server returns 429 (rate limit) errors. That could lead to infinite spamming (until user refreshed tab/tabs)
Co-authored-by: David Taylor <david@taylorhq.com>
Our fork was needed for OpenSSL 3 and Ruby 2.X compatibility.
The OpenSSL 3 part was merged into the gem for version 3.
Discourse dropped support for Ruby 2.X.
That means we don't need our fork anymore.
Co-authored-by: Rafael dos Santos Silva <xfalcox@gmail.com>
The `git` version in our discourse_test docker image was recently updated to include a permissions check before running any git commands. For this to pass, the owner of the discourse directory needs to match the user running any git commands.
Under GitHub actions, by default the working directory is created with uid=1000 as the owner. We run all our tests as `root`, so this mismatch causes git to raise the permissions error. We can't switch to run the entire workflow as the `discourse (uid=1000)` user because our discourse_test image is not configured to allow `discourse` access to postgres/redis directories. For now, this commit updates the working directory's owner to match the user running the workflow.
Under some situations, we would inadvertently return a public (unauthenticated) result to an authenticated API request. This commit adds the `Api-Key` header to our anonymous cache bypass logic.
The check used to be necessary because we validated the referrer too and
this bypass was a workaround a bug that is present in some browsers that
do not send the correct referrer.
When creating a group membership request, there is no character
limit on the 'reason' field. This can be potentially be used by
an attacker to create enormous amount of data in the database.
Only allow maximum of `50_000` characters for chat drafts. A hidden `max_chat_draft_length` setting can control this limit. A migration is also provided to delete any abusive draft in the database.
The number of drafts loaded on current user has also been limited and ordered by most recent update.
Note that spec files moved are not directly related to the fix.
When a user checks "Open all external links in a new tab" preference
he expects not to be overruled by unrelated text selections.
Yet if text is selected during a link click the link is followed on
the same tab. This change corrects that.
There was an issue where if hashtag-cooked HTML was sent
to the ExcerptParser without the keep_svg option, we would
end up with empty </use> and </svg> tags on the parts of the
excerpt where the hashtag was, in this case when a post
push notification was sent.
Fixed this, and also added a way to only display a plaintext
version of the hashtag for cases like this via PrettyText#excerpt.
When checking whether an existing upload should be secure
based on upload references, do not count deleted posts, since
there is still a reference attached to them. This can lead to
issues where e.g. an upload is used for a post then later on
a custom emoji.
Learn more about skidding here: https://popper.js.org/docs/v2/modifiers/offset/#skidding-1
This change has two goals:
- Fixes an issue when the user had zoomed the viewport and the popper would position on the opposite side
- Makes msg actions arguably more pleasant to the eye by preventing it to be right aligned with the message container
