ensure utf-8 nicknames on nickname GETs and user validator #1057

yeah, it’s probably best to add the UTF-8 check directly into the User.get_* functions, eventhough this unfortunately will also add the overhead to already known-good values. And rejecting insertion early in the validator is good to

Just some minor tweaks

yeah, it’s probably best to add the UTF-8 check directly into the `User.get_*` functions, eventhough this unfortunately will also add the overhead to already known-good values. And rejecting insertion early in the validator is good to Just some minor tweaks

lib/pleroma/user.ex

					
				@ -1223,3 +1227,3 @@

				  def get_cached_by_nickname_or_id(nickname_or_id, opts \\ []) do

				    restrict_to_local = Config.get([:instance, :limit_to_local_content])

				    if String.valid?(nickname_or_id) do

Since FlakeId can handle this and id-type arguments are presumably much more common it might be good to optimise this by only testing String.valid if there was no id match. E.g. something like:

with idlike <- is_integer(nickname_or_id) or FlakeId.flake_id?(nickname_or_id),
     nil <- if idlike, do: get_cached_by_id(nickname_or_id), else: nil,
     true <- String.valid?(nickname_or_id) do
  cond do
        restrict_to_local == false or not String.contains?(nickname_or_id, "@") ->
          get_cached_by_nickname(nickname_or_id)

        restrict_to_local == :unauthenticated and match?(%User{}, opts[:for]) ->
          get_cached_by_nickname(nickname_or_id)

        true ->
          nil
  end
else
  %User{} = user -> user
  _ -> nil
end

Though it might be nicer to split the "allowed to lookup" checks handled in the cond into a private function returning a boolean and then also handle this as just another with clause:

with idlike <- is_integer(nickname_or_id) or FlakeId.flake_id?(nickname_or_id),
     nil <- if idlike, do: get_cached_by_id(nickname_or_id), else: nil,
     true <- String.valid?(nickname_or_id),
     true <- allowed_to_lookup_nick(restrict_to_local, nickname_or_id, opts) do
  get_cached_by_nickname(nickname_or_id)
else
  %User{} = user -> user
  _ -> nil
end

Since `FlakeId` can handle this and id-type arguments are presumably much more common it might be good to optimise this by only testing `String.valid` if there was no id match. E.g. something like: ```elixir with idlike <- is_integer(nickname_or_id) or FlakeId.flake_id?(nickname_or_id), nil <- if idlike, do: get_cached_by_id(nickname_or_id), else: nil, true <- String.valid?(nickname_or_id) do cond do restrict_to_local == false or not String.contains?(nickname_or_id, "@") -> get_cached_by_nickname(nickname_or_id) restrict_to_local == :unauthenticated and match?(%User{}, opts[:for]) -> get_cached_by_nickname(nickname_or_id) true -> nil end else %User{} = user -> user _ -> nil end ``` Though it might be nicer to split the "allowed to lookup" checks handled in the `cond` into a private function returning a boolean and then also handle this as just another `with` clause: ```elixir with idlike <- is_integer(nickname_or_id) or FlakeId.flake_id?(nickname_or_id), nil <- if idlike, do: get_cached_by_id(nickname_or_id), else: nil, true <- String.valid?(nickname_or_id), true <- allowed_to_lookup_nick(restrict_to_local, nickname_or_id, opts) do get_cached_by_nickname(nickname_or_id) else %User{} = user -> user _ -> nil end ```

i just removed the check in nickname_by_id and let it live in the get_cached_by_nickname which should be suitable

floatingghost marked this conversation as resolved

lib/pleroma/user.ex

					
				@ -1248,0 +1256,4 @@

				        end

				    else

				      :error

				    end

Returning :error here breaks the documented type spec. It apparently still works fine anyway for /api/v1/accounts/:nick, but might still crash for other users of this function. I don’t think it’s necessary to distinguish this from a normal "not found" response though, after all no such user exists in the db and so just returning nil seems fine.

The same applies to the other two modified functions

Returning `:error` here breaks the documented type spec. It apparently still works fine anyway for `/api/v1/accounts/:nick`, but might still crash for other users of this function. I don’t think it’s necessary to distinguish this from a normal "not found" response though, after all no such user exists in the db and so just returning `nil` seems fine. The same applies to the other two modified functions

floatingghost marked this conversation as resolved

lib/pleroma/web/activity_pub/object_validators/user_validator.ex

					
				@ -84,2 +85,4 @@

				    end)

				  end

				  defp validate_nickname(%{"preferredUsername" => nick}) when is_binary(nick) do

what will happen if "preferredusername" exists but isn’t a binary?

what will happen if `"preferredusername"` exists but isn’t a binary?

updated to error if we have the preferredUsername key but its type is silly

Oneric marked this conversation as resolved

floatingghost added 1 commit

2026-02-07 19:09:16 +00:00

correct typings, duplicated check

ci/woodpecker/pr/test/1 Pipeline is pending approval

Details

ci/woodpecker/pr/test/2 Pipeline is pending approval

Details

58ee25bfbb

Oneric approved these changes

2026-02-07 19:35:44 +00:00

floatingghost merged commit cf8010a33e into develop

2026-02-07 19:41:27 +00:00

floatingghost deleted branch user-utf8

2026-02-07 19:41:27 +00:00

floatingghost referenced this pull request from a commit

2026-02-07 19:41:28 +00:00

Merge pull request 'ensure utf-8 nicknames on nickname GETs and user validator' (#1057) from user-utf8 into develop

Sign in to join this conversation.

No reviewers

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

AkkomaGang/akkoma!1057

Rows
Columns