Skip to content

Commit 6b27c5b

Browse files
committed
xmerl: Fix XPath evaluation bugs and xmerl_ucs:is_incharset/2 crash
* Fix XPath relational operators <, <= and >= * Fix XPath substring-before, string-length, sum, round and number() * Fix xmerl_ucs:is_incharset/2 crash on multi-character lists
1 parent daa9b36 commit 6b27c5b

4 files changed

Lines changed: 96 additions & 27 deletions

File tree

lib/xmerl/src/xmerl_ucs.erl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,7 @@ is_incharset(In,Charset) when is_list(In) ->
562562
{error,unsupported_charset};
563563
{error,_} ->
564564
false;
565-
[Int] when is_integer(Int) ->
565+
L when is_list(L) ->
566566
true
567567
end.
568568

lib/xmerl/src/xmerl_xpath_pred.erl

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -166,15 +166,15 @@ comp_expr('>', E1, E2, C) ->
166166
comp_expr('<', E1, E2, C) ->
167167
N1 = expr(E1,C),
168168
N2 = expr(E2,C),
169-
?boolean(compare_ineq_format(N1,N2,C) > compare_ineq_format(N2,N1,C));
169+
?boolean(compare_ineq_format(N1,N2,C) < compare_ineq_format(N2,N1,C));
170170
comp_expr('>=', E1, E2, C) ->
171171
N1 = expr(E1,C),
172172
N2 = expr(E2,C),
173-
?boolean(compare_ineq_format(N1,N2,C) > compare_ineq_format(N2,N1,C));
173+
?boolean(compare_ineq_format(N1,N2,C) >= compare_ineq_format(N2,N1,C));
174174
comp_expr('<=', E1, E2, C) ->
175175
N1 = expr(E1,C),
176176
N2 = expr(E2,C),
177-
?boolean(compare_ineq_format(N1,N2,C) > compare_ineq_format(N2,N1,C));
177+
?boolean(compare_ineq_format(N1,N2,C) =< compare_ineq_format(N2,N1,C));
178178
comp_expr('=', E1, E2, C) ->
179179
N1 = expr(E1,C),
180180
N2 = expr(E2,C),
@@ -508,8 +508,12 @@ contains(C, [A1, A2]) ->
508508
'substring-before'(C, [A1, A2]) ->
509509
S1 = mk_string(C, A1),
510510
S2 = mk_string(C, A2),
511-
Pos = string:str(S1, S2),
512-
?string(string:substr(S1, 1, Pos)).
511+
case string:str(S1, S2) of
512+
0 ->
513+
?string([]);
514+
Pos ->
515+
?string(string:substr(S1, 1, Pos - 1))
516+
end.
513517

514518
%% string: substring-after(string, string)
515519
'substring-after'(C, [A1, A2]) ->
@@ -536,10 +540,10 @@ substring(C, [A1, A2, A3]) ->
536540

537541
%% number: string-length(string?)
538542
'string-length'(C = #xmlContext{context_node = N}, []) ->
539-
length(mk_string(C, string_value(N)));
543+
?number(length(mk_string(C, string_value(N))));
540544

541545
'string-length'(C, [A]) ->
542-
length(mk_string(C, A)).
546+
?number(length(mk_string(C, A))).
543547

544548

545549
%% string: normalize-space(string?)
@@ -633,17 +637,17 @@ match_lang(_, _) ->
633637

634638
%% number: number(object)
635639
number(C = #xmlContext{context_node = N}, []) ->
636-
?number(mk_number(C, string(C, N)));
640+
?number(mk_number(C, string_value(N)));
637641
number(C, [Arg]) ->
638642
?number(mk_number(C, Arg)).
639643

640644

641645
sum(C, [Arg]) ->
642646
NS = mk_nodeset(C, Arg),
643-
lists:foldl(
644-
fun(N, Sum) ->
645-
Sum + mk_number(C, string(C, N))
646-
end, 0, NS).
647+
?number(lists:foldl(
648+
fun(N, Sum) ->
649+
Sum + mk_number(C, string_value(N))
650+
end, 0, NS)).
647651

648652
floor(C, [Arg]) ->
649653
Num = mk_number(C, Arg),
@@ -665,14 +669,14 @@ ceiling(C, [Arg]) ->
665669

666670

667671
round(C, [Arg]) ->
668-
case mk_number(C, Arg) of
672+
?number(case mk_number(C, Arg) of
669673
A when is_atom(A) ->
670674
A;
671675
N when is_integer(N) ->
672676
N;
673677
F when is_float(F) ->
674678
round(F)
675-
end.
679+
end).
676680

677681

678682
select_on_attribute([E = #xmlElement{attributes = Attrs}|T], K, V, Acc) ->

lib/xmerl/test/xmerl_SUITE.erl

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@
4545
all() ->
4646
[doctests,
4747
{group, cpd_tests}, xpath_text1, xpath_main,
48-
xpath_abbreviated_syntax, xpath_functions, xpath_namespaces,
48+
xpath_abbreviated_syntax, xpath_functions, xpath_relational,
49+
xpath_namespaces, ucs_is_incharset,
4950
{group, misc}, {group, eventp_tests},
5051
{group, ticket_tests}, {group, app_test},
5152
{group, appup_test}, {group, format_test}].
@@ -192,6 +193,10 @@ xpath_functions(Config) ->
192193
file:set_cwd(filename:join(datadir(Config),xpath)),
193194
ok = xpath_abbrev:functions().
194195

196+
xpath_relational(Config) ->
197+
file:set_cwd(filename:join(datadir(Config),xpath)),
198+
ok = xpath_abbrev:relational_operators().
199+
195200
xpath_namespaces(Config) ->
196201
file:set_cwd(filename:join(datadir(Config),xpath)),
197202
ok = xpath_abbrev:namespaces().
@@ -673,6 +678,16 @@ allow_entities_test(Config) ->
673678
(catch xmerl_scan:file(File, [{allow_entities, false}])),
674679
ok.
675680

681+
%% Regression: xmerl_ucs:is_incharset/2 crashed with a case_clause on a
682+
%% multi-character list for charsets handled via to_unicode/2 (e.g. utf-8),
683+
%% because the clause only matched a single-element result.
684+
ucs_is_incharset(_Config) ->
685+
true = xmerl_ucs:is_incharset("abc", 'utf-8'),
686+
true = xmerl_ucs:is_incharset("a", 'utf-8'),
687+
true = xmerl_ucs:is_incharset("abc", 'iso-8859-1'),
688+
false = xmerl_ucs:is_incharset([256, 257], 'iso-8859-1'),
689+
ok.
690+
676691
%%======================================================================
677692
%% Support Functions
678693
%%======================================================================

lib/xmerl/test/xmerl_SUITE_data/xpath/xpath_abbrev.erl

Lines changed: 61 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,27 @@
1-
%%%-------------------------------------------------------------------
2-
%%% File : xpath_abbrev.erl
3-
%%% Author : Bertil Karlsson <bertil@finrod>
4-
%%% Description :
5-
%%%
6-
%%% Created : 17 Jan 2006 by Bertil Karlsson <bertil@finrod>
7-
%%%-------------------------------------------------------------------
1+
%% %CopyrightBegin%
2+
%%
3+
%% SPDX-License-Identifier: Apache-2.0
4+
%%
5+
%% Copyright Ericsson AB 2026. All Rights Reserved.
6+
%%
7+
%% Licensed under the Apache License, Version 2.0 (the "License");
8+
%% you may not use this file except in compliance with the License.
9+
%% You may obtain a copy of the License at
10+
%%
11+
%% http://www.apache.org/licenses/LICENSE-2.0
12+
%%
13+
%% Unless required by applicable law or agreed to in writing, software
14+
%% distributed under the License is distributed on an "AS IS" BASIS,
15+
%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
%% See the License for the specific language governing permissions and
17+
%% limitations under the License.
18+
%%
19+
%% %CopyrightEnd%
20+
821
-module(xpath_abbrev).
922

1023
-export([test/0, check_node_set/2, ticket_6873/0, ticket_7496/0, functions/0]).
24+
-export([relational_operators/0]).
1125
-export([namespaces/0]).
1226

1327
-include_lib("common_test/include/ct.hrl").
@@ -61,7 +75,7 @@ test() ->
6175
Res22 = xmerl_xpath:string("blipp[@id and @test]",E),
6276
ok = check_node_set("blipp[@id and @test]",Res22).
6377

64-
check_node_set("blipp",[E1,E2,E3]) ->
78+
check_node_set("blipp",[E1,E2,E3]) ->
6579
ok = xml_element_name(E1,blipp),
6680
ok = xml_element_name(E2,blipp),
6781
ok = xml_element_name(E3,blipp),
@@ -106,7 +120,7 @@ check_node_set("/myBS_model/blipp[3]/blupp[2]",[E]) ->
106120
#xmlElement{name=blupp,
107121
attributes=[#xmlAttribute{name=att,value="bluppc2"}]}=E,
108122
ok;
109-
check_node_set("blipp//plopp",[#xmlElement{name=plopp},#xmlElement{name=plopp}]) ->
123+
check_node_set("blipp//plopp",[#xmlElement{name=plopp},#xmlElement{name=plopp}]) ->
110124
ok;
111125
check_node_set("//plopp",[E1,E2,E3]) ->
112126
ok = xml_element_name(E1,plopp),
@@ -218,6 +232,33 @@ ticket_7496() ->
218232
ok = Test(Doc3,"//*[starts-with(local-name(),'p')]",
219233
[parent,pet,pet]).
220234

235+
%% Regression test for the relational operators '<', '<=' and '>='.
236+
%% These three clauses of xmerl_xpath_pred:comp_expr/4 each carried the
237+
%% body of '>' (a copy-paste error), so every one evaluated as 'a > b'.
238+
%% The cases below fail on the buggy code (each selects [e,f] / [] instead
239+
%% of the expected node sets) and pass once the operators are correct.
240+
relational_operators() ->
241+
Test = fun(Doc, XPath, Exp) ->
242+
Result = xmerl_xpath:string(XPath, Doc),
243+
Exp = [Name || #xmlElement{name = Name} <- Result],
244+
ok
245+
end,
246+
{Doc,_} = xmerl_scan:string("<a><b/><c/><d/><e/><f/></a>"),
247+
248+
%% Relational predicates over position() (number vs number).
249+
ok = Test(Doc, "/a/*[position() < 3]", [b, c]),
250+
ok = Test(Doc, "/a/*[position() <= 3]", [b, c, d]),
251+
ok = Test(Doc, "/a/*[position() >= 3]", [d, e, f]),
252+
ok = Test(Doc, "/a/*[position() > 3]", [e, f]),
253+
254+
%% Constant relational predicates, independent of position(): each is a
255+
%% boolean that is true, so it selects every child.
256+
All = [b, c, d, e, f],
257+
ok = Test(Doc, "/a/*[2 < 3]", All),
258+
ok = Test(Doc, "/a/*[3 <= 3]", All),
259+
ok = Test(Doc, "/a/*[3 >= 3]", All),
260+
ok = Test(Doc, "/a/*[3 > 2]", All),
261+
ok.
221262

222263
functions() ->
223264
Test = fun(Doc, XPath, Exp) ->
@@ -234,7 +275,7 @@ functions() ->
234275
end|| Obj <- Result],
235276
ok
236277
end,
237-
Foo =
278+
Foo =
238279
"<foo>"
239280
" <bar>"
240281
" <name>Xml</name>"
@@ -257,6 +298,16 @@ functions() ->
257298
ok = Test(Doc,"/foo/bar[starts-with(name, 'X')]",[bar,bar]),
258299
ok = Test(Doc,"/foo/bar[value = string(1)]/value/text()",["1"]),
259300

301+
%% Regression tests: substring-before used to include the separator's
302+
%% first character (off-by-one); string-length and round returned bare
303+
%% numbers instead of #xmlObj{type=number}; and sum and number() (no
304+
%% args) called string/2 with a bare node, crashing on any non-empty
305+
%% node-set.
306+
ok = Test(Doc,"/foo/bar[substring-before(name, 'a') = 'Xp']",[bar]),
307+
ok = Test(Doc,"/foo/bar[string-length(name) = 3]",[bar]),
308+
ok = Test(Doc,"/foo/bar[round(value) = 2]",[bar]),
309+
ok = Test(Doc,"/foo[sum(bar/value) = 6]",[foo]),
310+
ok = Test(Doc,"/foo/bar/value[number() = 2]",[value]),
260311

261312
{Doc2,_}= xmerl_scan:file("purchaseOrder.xml"),
262313
ok = Test(Doc2,"//*[starts-with(local-name(),'c')]",
@@ -266,7 +317,6 @@ functions() ->
266317
ok = Test(Doc2,"//*[starts-with(name(),'{http://www.example.com/PO1')]",
267318
['apo:purchaseOrder','apo:comment']).
268319

269-
270320
namespaces() ->
271321
{Doc,_} = xmerl_scan:file("purchaseOrder.xml", [{namespace_conformant, true}]),
272322

0 commit comments

Comments
 (0)