Bug #8034
closedFile.expand_path('something', '~') do not include home path
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
Updated by luislavena (Luis Lavena) about 11 years ago
- Assignee changed from windows to luislavena (Luis Lavena)
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?
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);
}
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.