proxy-lookup.exe returns wrong result

[miimou]

• I was not able to find an open or closed issue matching what I'm seeing

Setup

• Which version of Git for Windows are you using? Is it 32-bit or 64-bit?

$ git --version --build-options
git version 2.36.0.windows.1
cpu: x86_64
built from commit: ea1e13f73339d57cbe81a0bae6fba669aaccf656
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon

• Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?

$ cmd.exe /c ver

Microsoft Windows [Version 10.0.19044.1645]

• What options did you set as part of the installation? Or did you choose the
  defaults?

$ cat /etc/install-options.txt
Editor Option: VIM
Custom Editor Path:
Default Branch Option:
Path Option: Cmd
SSH Option: OpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFAlways
Bash Terminal Option: MinTTY
Git Pull Behavior Option: Merge
Use Credential Manager: Enabled
Performance Tweaks FSCache: Enabled
Enable Symlinks: Disabled
Enable Pseudo Console Support: Disabled
Enable FSMonitor: Disabled

• Any other interesting things about your environment that might be related
  to the issue you're seeing?

  • As this issue occurs in proxy-lookup.exe, I have a proxy.pac set when I found this problem.

Details

• Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

  • I am using Bash

• What commands did you run to trigger this issue? If you can provide a
  Minimal, Complete, and Verifiable example
  this will help us understand the issue.

  • Make proxy.pac that specifies proxy for 192.168.0.0/24

    function FindProxyForURL(url, host) {
        if (isInNet(host, "192.168.0.0", "255.255.255.0")) {
        return "PROXY 192.168.0.1:8080";
        }
        return "DIRECT";
    }

  • Make it accessible via HTTP
    • for example, by Perl in Git Bash

      $ cd directory/that/have/proxy.pac/
      $ perl -MHTTP::Daemon -MHTTP::Status -e "my \$d=HTTP::Daemon->new(LocalPort=>3000);
        while(my \$c=\$d->accept){
          \$c->get_request;\$c->send_file_response('proxy.pac');
        \$c->close;undef(\$c)}"

      • Edited on April 30, 2022 for later reference,
        modified the code to close connection after sending file to avoid stall by Keep-Alive
  • Configure to use the proxy.pac
    • In "Local Area Network (LAN) Settings" > "Automatic configuration",
      • Turn on "Use automatic configuration script"
      • Enter "Address", for example: "http://localhost:3000/proxy.pac"
  • Use proxy-lookup.exe to lookup proxy for http://192.168.0.1/

    $ proxy-lookup.exe http://192.168.0.1/
    1
    $ proxy-lookup.exe -v http://192.168.0.1/
    URL: h, proxy: 1

• What did you expect to occur after running these commands?

  $ proxy-lookup.exe http://192.168.0.1/
  192.168.0.1:8080
  $ proxy-lookup.exe -v http://192.168.0.1/
  URL: http://192.168.0.1/, proxy: 192.168.0.1:8080

• What actually happened instead?

  $ proxy-lookup.exe http://192.168.0.1/
  1
  $ proxy-lookup.exe -v http://192.168.0.1/
  URL: h, proxy: 1

My thought

• In proxy-lookup.c lines 83 and 85,
  format string used in wprintf for wide character string is %s, but it should be %ls.
  • page for printf in cppreference.com says that
    %ls is for wchar_t* and %s is for char* in both printf and wprintf.
  • Microsoft Docs' page
    says that %s is for wchar_t* in wprintf, but it is marked as Microsoft-specific.
  • Git for Windows is built with mingw-w64, and
    mingw-w64's source code for printf family
    implements %s to be interpreted as char*.
• Wide character string (UTF-16LE in Windows) is parsed as narrow string, causing characters at second and latter position to be stripped.
• Changing like this worked in my environment.

   diff --git a/git-extra/proxy-lookup.c b/git-extra/proxy-lookup.c
   --- a/git-extra/proxy-lookup.c
   +++ b/git-extra/proxy-lookup.c
   @@ -80,9 +80,9 @@ int main(int argc, char **argv)
    			proxy = L"";
    
    			if (verbose)
   -				wprintf(L"URL: %s, proxy: %s\n", arg, proxy);
   +				wprintf(L"URL: %ls, proxy: %ls\n", arg, proxy);
    			else
   -				wprintf(L"%s\n", proxy);
   +				wprintf(L"%ls\n", proxy);
    		}
    
    		return 0;

[dscho]

 diff --git a/git-extra/proxy-lookup.c b/git-extra/proxy-lookup.c
 --- a/git-extra/proxy-lookup.c
 +++ b/git-extra/proxy-lookup.c
 @@ -80,9 +80,9 @@ int main(int argc, char **argv)
  			proxy = L"";
  
  			if (verbose)
 -				wprintf(L"URL: %s, proxy: %s\n", arg, proxy);
 +				wprintf(L"URL: %ls, proxy: %ls\n", arg, proxy);
  			else
 -				wprintf(L"%s\n", proxy);
 +				wprintf(L"%ls\n", proxy);
  		}
  
  		return 0;

Good find.

But let me ask: why is this a bug report and not a Pull Request in https://github.com/git-for-windows/build-extra? ;-)

[miimou]

why is this a bug report and not a Pull Request in https://github.com/git-for-windows/build-extra? ;-)

For two reasons:

• I am not sure whether my change is right, because I did not think I found the cause of problem.
  I could not find the change after proxy-lookup.exe was introduced which caused the problem.
• I am not experienced in using GitHub and I did not know whether it is acceptable to make pull request with such uncertain patch.

I am sorry to have troubled you.

[dscho]

I am not sure whether my change is right, because I did not think I found the cause of problem.

Oh, your change is correct all right!

I could not find the change after proxy-lookup.exe was introduced which caused the problem.

Most likely a change in behavior by mingw-w64 where %s in wprintf() changed behavior.

I am not experienced in using GitHub and I did not know whether it is acceptable to make pull request with such uncertain patch.

I welcome your contribution. Please open a Pull Request and I will work with you to get it integrated.