Skip to content

Commit

Permalink
ssh: fix decode uid, gid
Browse files Browse the repository at this point in the history
- adds missing list_to_integer conversion upon decoding
- fix version typo
- enhanced tests
  • Loading branch information
u3s committed Mar 13, 2024
1 parent 0863bd3 commit 1a12aa3
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 15 deletions.
7 changes: 2 additions & 5 deletions lib/ssh/src/ssh_xfer.erl
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ encode_As(Vsn, [{AName, X}|As], Flags, Acc) ->
encode_As(Vsn, As,Flags bor ?SSH_FILEXFER_ATTR_UIDGID,
[?uint32(X) | Acc]);
ownergroup when Vsn>=5 ->
X1 = list_to_binary(integer_to_list(X)), % TODO: check owner and group
X1 = integer_to_binary(X), % TODO: check owner and group
encode_As(Vsn, As,Flags bor ?SSH_FILEXFER_ATTR_OWNERGROUP,
[?binary(X1) | Acc]);
permissions ->
Expand Down Expand Up @@ -736,13 +736,11 @@ decode_As(Vsn, [{AName, AField}|As], R, Flags, Tail) ->
decode_As(Vsn, As, setelement(AField, R, X), Flags, Tail2);
ownergroup when ?is_set(?SSH_FILEXFER_ATTR_OWNERGROUP, Flags),Vsn>=5 ->
<<?UINT32(Len), Bin:Len/binary, Tail2/binary>> = Tail,
X = binary_to_list(Bin),
X = binary_to_integer(Bin),
decode_As(Vsn, As, setelement(AField, R, X), Flags, Tail2);

permissions when ?is_set(?SSH_FILEXFER_ATTR_PERMISSIONS,Flags),Vsn>=5->
<<?UINT32(X), Tail2/binary>> = Tail,
decode_As(Vsn, As, setelement(AField, R, X), Flags, Tail2);

permissions when ?is_set(?SSH_FILEXFER_ATTR_PERMISSIONS,Flags),Vsn=<3->
<<?UINT32(X), Tail2/binary>> = Tail,
R1 = setelement(AField, R, X),
Expand All @@ -757,7 +755,6 @@ decode_As(Vsn, [{AName, AField}|As], R, Flags, Tail) ->
_ -> unknown
end,
decode_As(Vsn, As, R1#ssh_xfer_attr { type=Type}, Flags, Tail2);

acmodtime when ?is_set(?SSH_FILEXFER_ATTR_ACMODTIME,Flags),Vsn=<3 ->
<<?UINT32(X), Tail2/binary>> = Tail,
decode_As(Vsn, As, setelement(AField, R, X), Flags, Tail2);
Expand Down
57 changes: 47 additions & 10 deletions lib/ssh/test/ssh_sftp_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@
-include_lib("common_test/include/ct.hrl").
-include_lib("kernel/include/file.hrl").
-include("ssh_test_lib.hrl").
% Default timetrap timeout
-include_lib("stdlib/include/assert.hrl").
%% Default timetrap timeout
-define(default_timeout, test_server:minutes(1)).

%%--------------------------------------------------------------------
Expand Down Expand Up @@ -563,26 +564,62 @@ links(Config) when is_list(Config) ->
retrieve_attributes(Config) when is_list(Config) ->
FileName = proplists:get_value(filename, Config),
SftpFileName = w2l(Config, FileName),

{Sftp, _} = proplists:get_value(sftp, Config),
{ok, FileInfo} = ssh_sftp:read_file_info(Sftp, SftpFileName),
{ok, NewFileInfo} = file:read_file_info(FileName),

%% TODO comparison. There are some differences now is that ok?
ct:log("SFTP: ~p FILE: ~p~n", [FileInfo, NewFileInfo]).
ct:log("ssh_sftp:read_file_info(~p): ~p~n"
"file:read_file_info(~p): ~p",
[SftpFileName, FileInfo, FileName, NewFileInfo]),
{ExpectedUid, ExpectedGid} =
case {os:type(), proplists:get_value(group,Config)} of
{{win32, _}, openssh_server} ->
%% Windows compiled Erlang is expected will return 0;
%% but when Erlang(Windows) client interacts with
%% OpenSSH server - value 1000 is received by client
%% over SFTP (because OpenSSH is compiled for Linux
%% and runs on WSL)
{1000, 1000};
_ ->
{FileInfo#file_info.uid, FileInfo#file_info.gid}
end,
?assertEqual(ExpectedUid, NewFileInfo#file_info.uid),
?assertEqual(ExpectedGid, NewFileInfo#file_info.gid),
ok.

%%--------------------------------------------------------------------
set_attributes(Config) when is_list(Config) ->
FileName = proplists:get_value(testfile, Config),
SftpFileName = w2l(Config, FileName),

{Sftp, _} = proplists:get_value(sftp, Config),
{ok,Fd} = file:open(FileName, write),
io:put_chars(Fd,"foo"),
ok = ssh_sftp:write_file_info(Sftp, SftpFileName, #file_info{mode=8#400}),
{error, eacces} = file:write_file(FileName, "hello again"),
ok = ssh_sftp:write_file_info(Sftp, SftpFileName, #file_info{mode=8#600}),
ok = file:write_file(FileName, "hello again").
TestWriting =
fun(FInfo) ->
ok = ssh_sftp:write_file_info(Sftp, SftpFileName,
FInfo#file_info{mode=8#400}),
{error, eacces} = file:write_file(FileName, "hello again"),
ok = ssh_sftp:write_file_info(Sftp, SftpFileName,
FInfo#file_info{mode=8#600}),
ok = file:write_file(FileName, "hello again")
end,
TestWriting(#file_info{}),
IsErlangServer =
fun() ->
TcGroupPath = proplists:get_value(tc_group_path, Config),
{_, Path} = lists:unzip(lists:flatten(TcGroupPath)),
lists:member(erlang_server, Path)
end,
case IsErlangServer() of
true ->
ct:log("Testing with writing a complete #file_info record"),
{ok, FileInfo} = file:read_file_info(SftpFileName),
TestWriting(FileInfo);
_ ->
%% with OpenSSH daemon started by other user above instruction end
%% up with permission denied
ok
end,
ok.

%%--------------------------------------------------------------------
file_owner_access(Config) when is_list(Config) ->
Expand Down

0 comments on commit 1a12aa3

Please sign in to comment.