Bug #8034: File.expand_path('something', '~') do not include home path

• Status: Closed
• Assignee: luislavena (Luis Lavena)
• Target version: -
• ruby -v: 1.9.3p362

[rap-kasta (Pavel Manylov)]

=begin
Next code works correctly only on ruby <= 1.9.3p362.
Tested on Windows XP, Windows 2008 with ruby 1.9.3p194 (works properly), ruby 1.9.3p362 (bug), ruby 2.0.0p0 (bug).

(({File.expand_path('something', '~') #=> "C:/path/to/current/dir/~/something"}))

However, next code works as it should in all tested rubies:

(({File.expand_path '~' # => "C:/Documents and Settings/Jack"
Dir.home # => "C:/Documents and Settings/Jack"}))
=end

[luislavena (Luis Lavena)]

• Assignee changed from windows to luislavena (Luis Lavena)

[luislavena (Luis Lavena)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r39697.
Pavel, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

Expand home directory when used in dir_string

• win32/file.c (rb_file_expand_path_internal): Expand home directory when
  used as second parameter (dir_string). [ruby-core:53168] [Bug #8034]
• test/ruby/test_file_exhaustive.rb: add test to verify.

[luislavena (Luis Lavena)]

Hello,

This has been fixed in trunk and backports for both 2.0.0 and 1.9.3 have been requested.

Thank you for your report.

[nobu (Nobuyoshi Nakada)]

=begin
Why is (({File.expand_path('something', '~whoever')})) ignored?
=end

[luislavena (Luis Lavena)]

=begin
nobu (Nobuyoshi Nakada) wrote:

=begin
Why is (({File.expand_path('something', '~whoever')})) ignored?

There is no test for that, I didn't see a test that verifies that behavior so I didn't add code for it.

Also, (({~user})) doesn't work as you expect on Windows.
=end

=end

[luislavena (Luis Lavena)]

• % Done changed from 100 to 50
• Status changed from Closed to Assigned

=begin
Nobu,

I'll add test for (({File.expand_path('something', '~whoever')})) and add support for it in ((|dir_string|)).

=end

[luislavena (Luis Lavena)]

• Assignee changed from luislavena (Luis Lavena) to h.shirosaki (Hiroshi Shirosaki)

Following patch solves the issue:

https://gist.github.com/luislavena/5130078

However there is lot of duplication between wpath and wdir checks, which is crying for a refactoring.

Shirosaki-san, what do you think?

[h.shirosaki (Hiroshi Shirosaki)]

• Assignee changed from h.shirosaki (Hiroshi Shirosaki) to luislavena (Luis Lavena)

Luis, thank you for your work.
xfree(wpath); would be needed before rb_raise(rb_eArgError, "can't find user %s"...
Indeed refactoring is better if possible.

[luislavena (Luis Lavena)]

• Assignee changed from luislavena (Luis Lavena) to h.shirosaki (Hiroshi Shirosaki)
• % Done changed from 50 to 70

=begin
Shirosaki-san,

I've refactored the code, would you mind take a look?

https://gist.github.com/luislavena/5148562

After applying the patch, it fixes the original issue, however now nobu-san added another test:

assert_nothing_raised(ArgumentError) { File.expand_path("/", UnknownUserHome) }

Which requires more conditions if (({fname})) is '/' and set (({ignore_dir})) accordingly, but I'm confused about the scenario.

I'm not sure if that scenario is portable and correct since 1.8.7 and 1.9.2 return ArgumentError:

C:\Users\Luis>ruby -ve "p File.expand_path('/', '~foo')"
ruby 1.8.7 (2012-10-12 patchlevel 371) [i386-mingw32]
-e:1:in `expand_path': can't find user ~foo (ArgumentError)
from -e:1

C:\Users\Luis>ruby -ve "p File.expand_path('/', '~foo')"
ruby 1.9.2p290 (2011-07-09) [i386-mingw32]
-e:1:in expand_path': can't find user foo (ArgumentError) from -e:1:in '

Thoughts?

=end

[nobu (Nobuyoshi Nakada)]

luislavena (Luis Lavena) wrote:

After applying the patch, it fixes the original issue, however now nobu-san added another test:

assert_nothing_raised(ArgumentError) { File.expand_path("/", UnknownUserHome) }

Which requires more conditions if (({fname})) is '/' and set (({ignore_dir})) accordingly, but I'm confused about the scenario.

Sorry, fixed the test, it should raise an exception on Windows, but the
latest test should not.

[h.shirosaki (Hiroshi Shirosaki)]

• Assignee changed from h.shirosaki (Hiroshi Shirosaki) to luislavena (Luis Lavena)

luislavena (Luis Lavena) wrote:

I've refactored the code, would you mind take a look?

https://gist.github.com/luislavena/5148562

Coding style is inconsistent at if/else. Otherwise, looks good to me. Thank you.

diff --git a/win32/file.c b/win32/file.c
index 53cf085..350f8da 100644
--- a/win32/file.c
+++ b/win32/file.c
@@ -333,14 +333,12 @@ get_user_from_path(wchar_t **wpath, int offset, UINT cp, UINT path_cp, rb_encodi
convert_wchar_to_mb(wuser, &user, &size, cp);

 /* convert to VALUE and set the path encoding */

• if (path_cp == INVALID_CODE_PAGE)
• {

• if (path_cp == INVALID_CODE_PAGE) {
  tmp = rb_enc_str_new(user, size, rb_utf8_encoding());
  result = rb_str_encode(tmp, rb_enc_from_encoding(path_encoding), 0, Qnil);
  rb_str_resize(tmp, 0);
  }

• else
• {

• else {
  result = rb_enc_str_new(user, size, path_encoding);
  }

[luislavena (Luis Lavena)]

• Status changed from Assigned to Closed
• % Done changed from 70 to 100

This issue was solved with changeset r39751.
Pavel, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

Refactor rb_file_expand_path_internal for dir_string corner cases

• win32/file.c (get_user_from_path): add internal function that retrieves
  username from supplied path (refactored).
• win32/file.c (rb_file_expand_path_internal): refactor expansion of user
  home to use get_user_from_path and cover dir_string corner cases.
  [ruby-core:53168] [Bug #8034]

[luislavena (Luis Lavena)]

h.shirosaki (Hiroshi Shirosaki) wrote:

Luis, thank you for your work.
xfree(wpath); would be needed before rb_raise(rb_eArgError, "can't find user %s"...
Indeed refactoring is better if possible.

Hiroshi,

I've pushed the refactored code as r39751 and corrected the style issue you mentioned. Thank you.

I noticed getting the user home directory when detected "~" in either wpath or wdir can be refactored too.

But that is for another ticket :)

Thank you.