From a2b09d302904c271949f37fcd709cb8ae62846c5 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Sat, 29 Jan 2022 16:29:24 +0100 Subject: [PATCH 1/2] Simplify the jid data structure Remove fields that should actually never be used. The idea is that the non-normalised parts MUST actually have never been used for comparison, and are therefore not really needed. If only for printing and using them in the `to` and `from` attributes of a stanza, clients SHOULD also stringprep anyway, so the fact that the server might provide non-normalised strings might be a problem for clients who're not enforced to normalise them. I've also been verifying how many other XMPP implementations tackle this issue, and I've seen many other servers, and also a bunch of client frameworks, simply capturing the parts and normalising them, and building data structures that keep only their normalised pieces. --- include/jid.hrl | 5 +---- src/jid.erl | 28 +++++++++------------------- test/jid_SUITE.erl | 15 +-------------- test/jid_gen.erl | 5 ++--- 4 files changed, 13 insertions(+), 40 deletions(-) diff --git a/include/jid.hrl b/include/jid.hrl index e6d86b8..f1572b8 100644 --- a/include/jid.hrl +++ b/include/jid.hrl @@ -1,7 +1,4 @@ --record(jid, {user = <<>> :: jid:user(), - server = <<>> :: jid:server(), - resource = <<>> :: jid:resource(), - luser = <<>> :: jid:luser(), +-record(jid, {luser = <<>> :: jid:luser(), lserver = <<>> :: jid:lserver(), lresource = <<>> :: jid:lresource() }). diff --git a/src/jid.erl b/src/jid.erl index 0493bbf..1f276e6 100644 --- a/src/jid.erl +++ b/src/jid.erl @@ -75,10 +75,7 @@ make(User, Server, Res) -> {_, error, _} -> error; {_, _, error} -> error; {LUser, LServer, LRes} -> - #jid{user = User, - server = Server, - resource = Res, - luser = LUser, + #jid{luser = LUser, lserver = LServer, lresource = LRes} end. @@ -95,10 +92,7 @@ make_bare(User, Server) -> {error, _} -> error; {_, error} -> error; {LUser, LServer} -> - #jid{user = User, - server = Server, - resource = <<>>, - luser = LUser, + #jid{luser = LUser, lserver = LServer, lresource = <<>>} end. @@ -108,10 +102,7 @@ make_bare(User, Server) -> Server :: lserver(), Resource :: lresource()) -> jid(). make_noprep(LUser, LServer, LResource) -> - #jid{user = LUser, - server = LServer, - resource = LResource, - luser = LUser, + #jid{luser = LUser, lserver = LServer, lresource = LResource}. @@ -157,8 +148,7 @@ from_binary(_) -> from_binary_noprep(J) when is_binary(J), byte_size(J) < ?XMPP_JID_SIZE_LIMIT -> case from_binary_nif(J) of {U, S, R} -> - #jid{user = U, server = S, resource = R, - luser = U, lserver = S, lresource = R}; + #jid{luser = U, lserver = S, lresource = R}; error -> error end; from_binary_noprep(_) -> @@ -185,8 +175,8 @@ to_binary({<<>>, Server}) -> <>; to_binary({Node, Server}) -> <>; -to_binary(#jid{user = User, server = Server, resource = Resource}) -> - to_binary({User, Server, Resource}); +to_binary(#jid{luser = LUser, lserver = LServer, lresource = LResource}) -> + to_binary({LUser, LServer, LResource}); to_binary(Jid) when is_binary(Jid) -> Jid. @@ -256,7 +246,7 @@ to_lus(error) -> (jid()) -> jid(); (error) -> error. to_bare(#jid{} = JID) -> - JID#jid{resource = <<>>, lresource = <<>>}; + JID#jid{lresource = <<>>}; to_bare({U, S, _R}) -> {U, S, <<>>}; to_bare(error) -> @@ -268,13 +258,13 @@ replace_resource(#jid{} = JID, Resource) -> case resourceprep(Resource) of error -> error; LResource -> - JID#jid{resource = Resource, lresource = LResource} + JID#jid{lresource = LResource} end. %% @doc Replaces the resource part of a jid with a new resource, but without normalisation -spec replace_resource_noprep(jid(), resource()) -> jid(). replace_resource_noprep(#jid{} = JID, LResource) -> - JID#jid{resource = LResource, lresource = LResource}. + JID#jid{lresource = LResource}. %% @equiv jid:to_bare(jid:from_binary(BinaryJid)) -spec binary_to_bare(binary()) -> jid() | error. diff --git a/test/jid_SUITE.erl b/test/jid_SUITE.erl index c930717..a06f2ed 100644 --- a/test/jid_SUITE.erl +++ b/test/jid_SUITE.erl @@ -23,7 +23,6 @@ groups() -> binary_noprep_to_jid_fails_with_empty_binary, make_jid_fails_on_binaries_that_are_too_long, make_is_independent_of_the_input_format, - make_noprep_and_make_have_equal_raw_jid, make_noprep_is_independent_of_the_input_format, jid_to_lower_fails_if_any_binary_is_invalid, jid_replace_resource_failes_for_invalid_resource, @@ -98,18 +97,6 @@ make_is_independent_of_the_input_format(_) -> jid:make(U,S,R) == jid:make({U,S,R})), run_property(Prop, 100, 1, 500). -make_noprep_and_make_have_equal_raw_jid(_) -> - Prop = ?FORALL({U, S, R}, - {jid_gen:username(), jid_gen:domain(), jid_gen:resource()}, - begin - #jid{user = U, server = S, resource = R} = jid:make(U, S, R), - #jid{user = U, server = S, resource = R} = jid:make_noprep(U, S, R), - true - end - ), - run_property(Prop, 10, 1, 500). - - make_noprep_is_independent_of_the_input_format(_) -> Prop = ?FORALL({U, S, R}, {jid_gen:username(), jid_gen:domain(), jid_gen:resource()}, @@ -330,7 +317,7 @@ binary_to_jid3(<<>>, N, S, R) -> to_binary(Jid) when is_binary(Jid) -> % sometimes it is used to format error messages Jid; -to_binary(#jid{user = User, server = Server, resource = Resource}) -> +to_binary(#jid{luser = User, lserver = Server, lresource = Resource}) -> to_binary({User, Server, Resource}); to_binary({User, Server}) -> to_binary({User, Server, <<>>}); diff --git a/test/jid_gen.erl b/test/jid_gen.erl index 9c0c29a..772b2b6 100644 --- a/test/jid_gen.erl +++ b/test/jid_gen.erl @@ -23,15 +23,14 @@ jid_struct() -> ?LET({U, S, R}, {username(), domain(), resource()}, - {jid, U, S, R, U, S, R}). + {jid, U, S, R}). from_jid() -> oneof([ {jid_gen:username(), jid_gen:domain(), jid_gen:resource()}, {<<>>, jid_gen:domain(), jid_gen:resource()}, {jid_gen:username(), jid_gen:domain()}, - {jid, jid_gen:username(), jid_gen:domain(), jid_gen:resource(), - jid_gen:username(), jid_gen:domain(), jid_gen:resource()} + {jid, jid_gen:username(), jid_gen:domain(), jid_gen:resource()} ]). jid() -> From 648f1c9ea0163195b3c8c734a3a335410dd15412 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Sat, 16 Apr 2022 20:17:21 +0200 Subject: [PATCH 2/2] Add to_bare_binary, which is a very common construct in MIM --- src/jid.erl | 39 +++++++++++++++++++-------------------- test/jid_SUITE.erl | 6 ++++++ 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/jid.erl b/src/jid.erl index 1f276e6..c0f56b4 100644 --- a/src/jid.erl +++ b/src/jid.erl @@ -16,26 +16,13 @@ -module(jid). -on_load(load/0). --export([make/3]). --export([make/1]). --export([make_bare/2]). --export([make_noprep/3]). --export([make_noprep/1]). --export([are_equal/2]). --export([are_bare_equal/2]). --export([from_binary/1]). --export([from_binary_noprep/1]). --export([to_binary/1]). --export([is_nodename/1]). --export([nodeprep/1]). --export([nameprep/1]). --export([resourceprep/1]). --export([to_lower/1]). --export([to_lus/1]). --export([to_bare/1]). --export([replace_resource/2]). --export([replace_resource_noprep/2]). --export([binary_to_bare/1]). +-export([make/3, make/1, make_bare/2, make_noprep/3, make_noprep/1]). +-export([from_binary/1, from_binary_noprep/1]). +-export([binary_to_bare/1, to_bare_binary/1, to_binary/1]). +-export([are_equal/2, are_bare_equal/2, is_nodename/1]). +-export([nodeprep/1, nameprep/1, resourceprep/1]). +-export([to_lower/1, to_lus/1, to_bare/1]). +-export([replace_resource/2, replace_resource_noprep/2]). -export([str_tolower/1]). -include("jid.hrl"). @@ -180,6 +167,18 @@ to_binary(#jid{luser = LUser, lserver = LServer, lresource = LResource}) -> to_binary(Jid) when is_binary(Jid) -> Jid. +-spec to_bare_binary(simple_jid() | simple_bare_jid() | jid() | literal_jid()) -> binary() | error. +to_bare_binary({<<>>, Server}) -> + <>; +to_bare_binary({User, Server}) -> + <>; +to_bare_binary({User, Server, _}) -> + to_bare_binary({User, Server}); +to_bare_binary(#jid{luser = LUser, lserver = LServer}) -> + to_bare_binary({LUser, LServer}); +to_bare_binary(Jid) when is_binary(Jid) -> + binary_to_bare(Jid). + %% @doc Returns true if the input is a valid user part -spec is_nodename(<<>> | binary()) -> boolean(). is_nodename(<<>>) -> diff --git a/test/jid_SUITE.erl b/test/jid_SUITE.erl index a06f2ed..5960e4a 100644 --- a/test/jid_SUITE.erl +++ b/test/jid_SUITE.erl @@ -39,6 +39,7 @@ groups() -> compare_bare_jids_doesnt_depend_on_the_order, compare_bare_with_jids_structs_and_bare_jids, binary_to_bare_equals_binary_and_then_bare, + to_bare_binary_equals_to_bare_then_to_binary, to_lower_to_bare_equals_to_bare_to_lower, make_to_lus_equals_to_lower_to_lus, make_bare_like_make_with_empty_resource @@ -233,6 +234,11 @@ binary_to_bare_equals_binary_and_then_bare(_) -> equals(jid:to_bare(jid:from_binary(A)), jid:binary_to_bare(A))), run_property(Prop, 200, 1, 100). +to_bare_binary_equals_to_bare_then_to_binary(_) -> + Prop = ?FORALL(A, jid_gen:jid_struct(), + equals(jid:to_binary(jid:to_bare(A)), jid:to_bare_binary(A))), + run_property(Prop, 200, 1, 100). + to_lower_to_bare_equals_to_bare_to_lower(_) -> Prop = ?FORALL(JID, oneof([jid_gen:jid_struct(), {jid_gen:maybe_valid_username(),