AkkomaGang/akkoma
AkkomaGang/akkoma
17
70
Fork 115
Code Issues 147 Pull requests 14 Projects 2 Releases 23 Packages Wiki Activity

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

Merged
floatingghost merged 4 commits from user-utf8 into develop 2026-02-07 19:41:27 +00:00
Conversation 2 Commits 4 Files changed 3 +16 -16
2 changed files with 16 additions and 16 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 58ee25bfbb - Show all commits

correct typings, duplicated check
Some checks are pending
ci/woodpecker/pr/test/1 Pipeline is pending approval
Details
ci/woodpecker/pr/test/2 Pipeline is pending approval
Details

Prev Next
FloatingGhost 2026-02-07 19:09:02 +00:00
commit 58ee25bfbb

28
lib/pleroma/user.ex
View file

@ -1221,29 +1221,25 @@ defmodule Pleroma.User do
end
end)
else
:error
nil
end
end
def get_cached_by_nickname_or_id(nickname_or_id, opts \\ []) do
if String.valid?(nickname_or_id) do
restrict_to_local = Config.get([:instance, :limit_to_local_content])
restrict_to_local = Config.get([:instance, :limit_to_local_content])
floatingghost marked this conversation as resolved
Oneric commented 2026-01-25 15:32:19 +00:00
Copy link

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 ```
floatingghost commented 2026-02-07 19:10:19 +00:00
Copy link

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

i just removed the check in nickname_by_id and let it live in the get_cached_by_nickname which should be suitable
cond do
is_integer(nickname_or_id) or FlakeId.flake_id?(nickname_or_id) ->
get_cached_by_id(nickname_or_id) || get_cached_by_nickname(nickname_or_id)
cond do
is_integer(nickname_or_id) or FlakeId.flake_id?(nickname_or_id) ->
get_cached_by_id(nickname_or_id) || get_cached_by_nickname(nickname_or_id)
restrict_to_local == false or not String.contains?(nickname_or_id, "@") ->
get_cached_by_nickname(nickname_or_id)
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)
restrict_to_local == :unauthenticated and match?(%User{}, opts[:for]) ->
get_cached_by_nickname(nickname_or_id)
true ->
nil
end
else
:error
true ->
nil
end
end
@ -1255,7 +1251,7 @@ defmodule Pleroma.User do
Repo.get_by(User, nickname: local_nickname(nickname))
end
else
:error
nil
end
end

4
lib/pleroma/web/activity_pub/object_validators/user_validator.ex
View file

@ -93,5 +93,9 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidators.UserValidator do
end
end
defp validate_nickname(%{"preferredUsername" => _nick}) do
{:error, "Nickname is not a valid string"}
end
defp validate_nickname(_), do: :ok
end