[PUP-5736] Investigate use of win32-process gem with unicode paths Created: 2016/01/21  Updated: 2017/04/12  Resolved: 2016/02/01

Status: Closed
Project: Puppet
Component/s: None
Affects Version/s: None
Fix Version/s: PUP 4.4.0

Type: Task Priority: Normal
Reporter: Ethan Brown Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: i18n, utf-8
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
relates to PUP-5752 Replace win32-process calls with inte... Resolved
relates to PUP-7445 Remove win32-process usage within Puppet Resolved
relates to PUP-5798 Puppet::Util::Execution.execute does ... Closed
relates to PUP-5780 Create minor patch for Win32-Process gem Closed
relates to PUP-5782 Re-vendor Win32-Process gem inside of... Closed
relates to PUP-5781 Verify new Win32-Process gem includes... Closed
relates to PUP-5560 Look at Improving Unicode for all Win... Resolved
Epic Link: Non-US Language / Unicode Support for Windows
Story Points: 1
Sprint: Windows 2016-02-10
Release Notes: Not Needed


For starters, the affected parts appear to be:

However, note that Win32::Process actually monkey patches the process object - https://github.com/djberg96/win32-process/blob/ffi/lib/win32/process.rb

So we'll also want to look for calls to these functions as well:

Comment by Glenn Sarti [ 2016/01/26 ]

Also, watch out for;

Comment by Glenn Sarti [ 2016/01/29 ]

kill function does not call any non wide string functions and doesn't appear to have any way to input unicode strings.

Comment by Glenn Sarti [ 2016/01/29 ]

create function already calls CreateProcessWithLogonW and CreateProcessW. However we don't appear to have any tests for unicode based paths. Need to add these.

Comment by Glenn Sarti [ 2016/01/29 ]

setpriority does not use any non-wide functions and doesn't appear to have any to input unicode strings as parameters

Comment by Glenn Sarti [ 2016/01/29 ]

uid function does call ANSI API calls but Puppet does not appear to ever call uid for Windows.

Comment by Glenn Sarti [ 2016/01/29 ]

Apart from one integration test there isn't much to do here. I suppose Puppet::Util::Windows::Process could completely shadow Win32::Process and then all Puppet calls go through that to make it easier to segregate Win32/process but I'm not sure it's worth it.

Comment by Ethan Brown [ 2016/02/01 ]

Yeah, not worth wholesale replacing if we're not exposed (I was mostly concerned about Unicode paths to binaries for create).

As we talked about a little bit, I think there are a few things we should do (as a lower priority thing):

Comment by Glenn Sarti [ 2016/02/01 ]

Raised three tickets for the tasks mentioned by Ethan

PUP-5780, PUP-5781 and PUP-5782

Comment by Glenn Sarti [ 2016/02/01 ]

Changing Story Points to 1 as it just required 3 tickets to be raised and a few integration tests to be written.
Adding to the current sprint

Comment by Glenn Sarti [ 2016/02/01 ]

The parent investigation ticket was closed as it was not required. Instead a few integration tests are to be created to ensure against regression failures

Comment by Glenn Sarti [ 2016/02/01 ]

PR Raised

Generated at Sun Sep 27 17:29:40 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.