grke / burp

burp - backup and restore program
http://burp.grke.net
Other
483 stars 77 forks source link

Utest on windows fails for test_getdatestr() #705

Closed deajan closed 6 years ago

deajan commented 6 years ago

For what it worth, there's an error in the windows version of the unit tests.

C:\Users\Me\Desktop>utest
Running suite(s): alloc
 base64
 client_auth
 client_monitor_lline
 client_protocol2_rabin_read
 cmd

Index of symbols

  a: Append to a file
  b: Backup timestamp
  c: Generic command
  d: Directory
  e: Error message
  f: Plain file
  g: ----------------
  h: ----------------
  i: Interrupt
  j: ----------------
  k: Windows EFS file
  l: Soft link
  m: Extra meta data
  n: Encrypted meta data
  o: ----------------
  p: Message
  q: Save path part of a signature
  r: File attribute information
  s: Special file - fifo, socket, device node
  t: Path to data on the server
  u: Windows VSS footer
  v: Windows VSS header
  w: Warning
  x: End of file transmission
  y: Encrypted file
  z: Plain file changed
  A: ----------------
  B: Block data
  C: ----------------
  D: Request for block of data
  E: Timestamp now/end
  F: Fingerprint part of a signature
  G: Bytes estimated
  H: ----------------
  I: ----------------
  J: ----------------
  K: ----------------
  L: Hard link
  M: Path to a manifest
  N: ----------------
  O: Bytes
  P: Bytes received
  Q: Bytes sent
  R: File attribute information preceding block signatures
  S: Block signature
  T: ----------------
  U: Encrypted windows VSS footer
  V: Encrypted windows VSS header
  W: Control packet
  X: ----------------
  Y: Total counter
  Z: Grand total counter

 conf
 fzp
 hexmap
 pathcmp
 protocol1_handy
 protocol1_rs_buf
 protocol2_blist
 protocol2_blk
 protocol2_rabin_rabin
 protocol2_rabin_rconf
 protocol2_rabin_win
 sbuf_protocol2
 slist
 times
98%: Checks: 62, Failures: 1, Errors: 0
/opt/burp/2.1.28/burp-2.1.28/utest/test_times.c:55:F:Core:test_getdatestr:0: Assertion '!strcmp(ds[i].str, str)' failed
grke commented 6 years ago

Hello, I run this every time I build a Windows installer and it passes. Maybe your Windows computer is speaking something other than English? If you want to debug it, you can add printf() lines to utest/test_times.c, just before line 55: printf("expected: '%s', got: '%s'\n", ds[i].str, str);

deajan commented 6 years ago

Indeed, my computer is a French :) I guess all functions where getdatestr is used should be faulty on non English Windows. This can't be true since this would mean that any date comparaisons would be false. I'll definitly look that up in a next compile session.

grke commented 6 years ago

I think it just means that my tests fail because my tests expect English text. It doesn't mean that the functions are failing. The tests should probably just temporarily set English locales.

deajan commented 6 years ago

Added the debug line, here's my output

expected: 'never', got: 'never'
expected: '1970-01-01 00:16:40', got: '1970-01-01 01:16:40'
98%: Checks: 62, Failures: 1, Errors: 0
/opt/burp/2.1.28/burp-2.1.28/utest/test_times.c:56:F:Core:test_getdatestr:0: Assertion '!strcmp(ds[i].str, str)' failed

I'm using UTC+1. If I happen to use UTC+0, I don't have errors.

grke commented 6 years ago

In that test, I temporarily set the TZ environment variable on non-Windows, but not for Windows.

Do you know how the test can do a similar thing for Windows?

grke commented 6 years ago

Found this, will try it: https://msdn.microsoft.com/en-us/library/windows/desktop/ms686206(v=vs.85).aspx

grke commented 6 years ago

I couldn't get SetEnvironmentVariable to do what I wanted. However, utest.exe runs fine when run from inside cmd.exe like this:

SET TZ=UTC+0
utest.exe

Or from within cygwin like this:

TZ=UTC+0 utest.exe

I will close this issue now, as I think everything is basically OK.

deajan commented 6 years ago

Indeed, I just hope that clients and servers in different timezones won't be affected by any time comparaison issues that may happen.

Btw, works for Windows (in batch hell)

tzutil /g > %TMP%\TZ && set /p TZ=<%TMP%\TZ
tzutil /s UTC
utest.exe
tzutil /g "%TZ%"
grke commented 6 years ago

It should be OK because time comparisons only happen server side.