Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCP receive file size error on windows. #109

Closed
xiaoniu-578fa6bff964d005 opened this issue Mar 14, 2019 · 6 comments · Fixed by #121
Closed

SCP receive file size error on windows. #109

xiaoniu-578fa6bff964d005 opened this issue Mar 14, 2019 · 6 comments · Fixed by #121

Comments

@xiaoniu-578fa6bff964d005
Copy link

xiaoniu-578fa6bff964d005 commented Mar 14, 2019

When I download a file of 4 bytes size, ScpFileStat.size() shows 1552549416, which is actually current time.

Some debug indicate that this issue is caused by different layout between libc::windows::stat and stat.

Here are the interpretations of same memory block for libc::windows::stat:
图片
and for stat:
图片

The libssh2_struct_stat have same layout with libc::windows::stat, however the input argument of libssh2_scp_recv in libssh2-sys is of type stat, ref.

Here is the memory layout of libc::windows::stat on my computer:
type: libc::windows::stat: 56 bytes, alignment: 8 bytes
field .st_dev: 4 bytes
field .st_ino: 2 bytes
field .st_mode: 2 bytes
field .st_nlink: 2 bytes
field .st_uid: 2 bytes
field .st_gid: 2 bytes
padding: 2 bytes
field .st_rdev: 4 bytes, alignment: 4 bytes
padding: 4 bytes
field .st_size: 8 bytes, alignment: 8 bytes
field .st_atime: 8 bytes
field .st_mtime: 8 bytes
field .st_ctime: 8 bytes
Size of stat: 48 bytes.

My environment:
rustc 1.32.0 (9fda7c223 2019-01-16)
stable-x86_64-pc-windows-msvc and stable-x86_64-pc-windows-gnu(with MinGW-W64)

cargo.lock:
libssh2-sys 0.2.11
libc 0.2.50

@xiaoniu-578fa6bff964d005
Copy link
Author

xiaoniu-578fa6bff964d005 commented Mar 14, 2019

It confuse me a lot why stat and libssh2_struct_stat have different size.
According to this line they should be same.

@alexcrichton
Copy link
Owner

This may be a case where stat vs stat64 or something like that is causing problems, and this is likely a bug in the libssh2-sys crate itself.

If you can checkout the repository and run cargo run in systest it should alert any differences between the two

@valerius21
Copy link

valerius21 commented Apr 27, 2019

Unfortunately, it does not.

warning: cl : Command line warning D9025 : overriding '/W4' with '/W3'
warning: use of deprecated item 'std::sync::atomic::ATOMIC_BOOL_INIT': the `new` function is now preferred
 --> C:\Users\Valerius\Desktop\_ssh\ssh2-rs\target\debug\build\systest-07db2a0d3075e19a\out/all.rs:4:49
  |
4 |             use std::sync::atomic::{AtomicBool, ATOMIC_BOOL_INIT, Ordering};
  |                                                 ^^^^^^^^^^^^^^^^
  |
  = note: #[warn(deprecated)] on by default

warning: use of deprecated item 'std::sync::atomic::ATOMIC_USIZE_INIT': the `new` function is now preferred
 --> C:\Users\Valerius\Desktop\_ssh\ssh2-rs\target\debug\build\systest-07db2a0d3075e19a\out/all.rs:5:50
  |
5 |             use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT};
  |                                                  ^^^^^^^^^^^^^^^^^

warning: use of deprecated item 'std::sync::atomic::ATOMIC_BOOL_INIT': the `new` function is now preferred
  --> C:\Users\Valerius\Desktop\_ssh\ssh2-rs\target\debug\build\systest-07db2a0d3075e19a\out/all.rs:41:41
   |
41 |             static FAILED: AtomicBool = ATOMIC_BOOL_INIT;
   |                                         ^^^^^^^^^^^^^^^^
help: use of deprecated item 'std::sync::atomic::ATOMIC_BOOL_INIT': the `new` function is now preferred
   |
41 |             static FAILED: AtomicBool = AtomicBool::new(false);
   |                                         ^^^^^^^^^^^^^^^^^^^^^^

warning: use of deprecated item 'std::sync::atomic::ATOMIC_USIZE_INIT': the `new` function is now preferred
  --> C:\Users\Valerius\Desktop\_ssh\ssh2-rs\target\debug\build\systest-07db2a0d3075e19a\out/all.rs:42:42
   |
42 |             static NTESTS: AtomicUsize = ATOMIC_USIZE_INIT;
   |                                          ^^^^^^^^^^^^^^^^^
help: use of deprecated item 'std::sync::atomic::ATOMIC_USIZE_INIT': the `new` function is now preferred
   |
42 |             static NTESTS: AtomicUsize = AtomicUsize::new(0);
   |                                          ^^^^^^^^^^^^^^^^^^^

    Finished dev [unoptimized + debuginfo] target(s) in 1m 04s
     Running `C:\Users\Valerius\Desktop\_ssh\ssh2-rs\target\debug\systest.exe`
RUNNING ALL TESTS
PASSED 769 tests

It does compile just fine on Windows 10, however the issue remains.

@JoyceBabu
Copy link
Contributor

JoyceBabu commented Jul 22, 2019

I am also seeing the same issue. ScpFileStat::size is returning the timestamp instead of the actual filesize.

@JoyceBabu
Copy link
Contributor

On 64 bit build it is returning the timestamp. But on 32 bit build, it is returning an even bigger number.

For a file of size 40960 bytes, I am getting the following sizes depending on the target architecture.

64 Bit - 1563919174
32 Bit - 6716986311686195062

@JoyceBabu
Copy link
Contributor

JoyceBabu commented Jul 24, 2019

The issue happens when libssh2 is compiled with large file support for windows enabled. When it is enabled, libssh2_struct_stat is an alias for _stati64.

I found the following comment in libssh2/include/libssh2.h

/* libssh2_scp_recv is DEPRECATED, do not use! */
LIBSSH2_API LIBSSH2_CHANNEL *libssh2_scp_recv(LIBSSH2_SESSION *session,
                                              const char *path,
                                              struct stat *sb);
/* Use libssh2_scp_recv2 for large (> 2GB) file support on windows */
LIBSSH2_API LIBSSH2_CHANNEL *libssh2_scp_recv2(LIBSSH2_SESSION *session,
                                               const char *path,
                                               libssh2_struct_stat *sb);

So I tried updating sess.scp_recv to use libssh2_scp_recv2 instead of the deprecated libssh2_scp_recv, and now it is working correctly on both Windows and macOS.

wez added a commit to wez/ssh2-rs that referenced this issue Jul 29, 2019
* Adopt scp_recv2 instead, which uses compatible 64-bit stat types
* Mark scp_recv as deprecated
* small version bump

Fixes alexcrichton#109
Refs alexcrichton#117

Co-authored-by: Joyce Babu <[email protected]>
wez added a commit to wez/ssh2-rs that referenced this issue Jul 31, 2019
* Adopt scp_recv2 instead, which uses compatible 64-bit stat types
* Mark scp_recv as deprecated
* small version bump

Fixes alexcrichton#109
Refs alexcrichton#117

Co-authored-by: Joyce Babu <[email protected]>
@wez wez closed this as completed in #121 Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants