Project

General

Profile

Actions

Bug #8034

closed

File.expand_path('something', '~') do not include home path

Added by rap-kasta (Pavel Manylov) about 11 years ago. Updated about 11 years ago.

Status:
Closed
Target version:
-
ruby -v:
1.9.3p362
Backport:
[ruby-core:53168]

Description

=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


Related issues 2 (0 open2 closed)

Related to Backport200 - Backport #8069: Backport r39697Closednagachika (Tomoyuki Chikanaga)03/10/2013Actions
Related to Backport193 - Backport #8070: Backport r39697Closedusa (Usaku NAKAMURA)03/10/2013Actions

Updated by luislavena (Luis Lavena) about 11 years ago

  • Assignee changed from windows to luislavena (Luis Lavena)
Actions #2

Updated by luislavena (Luis Lavena) about 11 years ago

  • 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.

Updated by luislavena (Luis Lavena) about 11 years ago

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.

Updated by nobu (Nobuyoshi Nakada) about 11 years ago

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

Updated by luislavena (Luis Lavena) about 11 years ago

=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

Updated by luislavena (Luis Lavena) about 11 years ago

  • % 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

Updated by luislavena (Luis Lavena) about 11 years ago

  • 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?

Updated by h.shirosaki (Hiroshi Shirosaki) about 11 years ago

  • 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.

Updated by luislavena (Luis Lavena) about 11 years ago

  • 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

Updated by nobu (Nobuyoshi Nakada) about 11 years ago

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.

Updated by h.shirosaki (Hiroshi Shirosaki) about 11 years ago

  • 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);
    }
Actions #12

Updated by luislavena (Luis Lavena) about 11 years ago

  • 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]

Updated by luislavena (Luis Lavena) about 11 years ago

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.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0