Bug #5530

SEEK_SET malfunctions when used with 'append' File.open mode

Added by Joshua J. Drake almost 4 years ago. Updated almost 3 years ago.

[ruby-core:40573]
Status:Closed
Priority:Normal
Assignee:Motohiro KOSAKI
ruby -v:all Backport:

Description

The following code demonstrates the issue. As documented, IO#seek says "SEEK_SET" will move to a position relative to the start of the file. Using 'ab', it doesn't actually seek where it should. It's possible the documentation is just wrong here, either the code or documentation should change though.

#!/usr/bin/env ruby

modes = [ 'ab', 'wb', 'w+b', 'r+b' ]

filename = '/tmp/test.txt'
desired = "AABBBBAA"

modes.each { |mode|
File.unlink(filename) rescue nil
File.open(filename, "wb") { |fd|
fd.write('AAAAAAAA')
}

    File.open(filename, mode) { |fd|
            fd.seek(2, IO::SEEK_SET)
            fd.write("BBBB")
            fd.close
    }

    data = ''
    File.open(filename, "rb") { |fd|
            data = fd.read(fd.stat.size)
    }

    puts "%3s #{data.inspect}" % mode

}

File.unlink(filename)
puts ""

Output:

fear:0:ruby$ rvm all ruby -v file_mode_test.rb
ruby 1.8.6 (2010-09-02 patchlevel 420) [x86_64-linux]
ab "AAAAAAAABBBB"
wb "\000\000BBBB"
w+b "\000\000BBBB"
r+b "AABBBBAA"

ruby 1.8.7 (2011-02-18 patchlevel 334) [x86_64-linux]
ab "AAAAAAAABBBB"
wb "\000\000BBBB"
w+b "\000\000BBBB"
r+b "AABBBBAA"

ruby 1.9.1p378 (2010-01-10 revision 26273) [x86_64-linux]
ab "AAAAAAAABBBB"
wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

ruby 1.9.2p290 (2011-07-09 revision 32553) [x86_64-linux]
ab "AAAAAAAABBBB"
wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

ruby 1.8.7 (2011-06-30 patchlevel 352) [x86_64-linux]
ab "AAAAAAAABBBB"
wb "\000\000BBBB"
w+b "\000\000BBBB"
r+b "AABBBBAA"

ruby 1.9.2p180 (2011-02-18 revision 30909) [x86_64-linux]
ab "AAAAAAAABBBB"
wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

ruby 1.9.1p431 (2011-02-18 revision 30908) [x86_64-linux]
Error loading gem paths on load path in gem_prelude
can't modify frozen string
internal:gem_prelude:69:in force_encoding'
<internal:gem_prelude>:69:in
set_home'
internal:gem_prelude:38:in dir'
<internal:gem_prelude>:76:in
set_paths'
internal:gem_prelude:47:in path'
<internal:gem_prelude>:286:in
push_all_highest_version_gems_on_load_path'
internal:gem_prelude:355:in `'
ab "AAAAAAAABBBB"
wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

ruby 1.9.3dev (2011-07-31 revision 32789) [x86_64-linux]
ab "AAAAAAAABBBB"
wb "\x00\x00BBBB"
w+b "\x00\x00BBBB"
r+b "AABBBBAA"

fear:0:ruby$

The other modes behave as expected.

See also - http://dev.metasploit.com/redmine/issues/3199

file-const.patch Magnifier (1.43 KB) Motohiro KOSAKI, 11/19/2012 04:59 PM

file_constants_null.patch Magnifier (704 Bytes) Zachary Scott, 11/20/2012 02:45 PM

Associated revisions

Revision 37746
Added by Motohiro KOSAKI almost 3 years ago

  • io.c (Init_IO): removed all rb_file_const() into file.c.
  • file.c (Init_File): replace with rb_file_const() with rb_define_const() because RDoc don't care rb_file_const. [Bug #5530]

Revision 37746
Added by Motohiro KOSAKI almost 3 years ago

  • io.c (Init_IO): removed all rb_file_const() into file.c.
  • file.c (Init_File): replace with rb_file_const() with rb_define_const() because RDoc don't care rb_file_const. [Bug #5530]

History

#1 Updated by Motohiro KOSAKI almost 4 years ago

The code is correct. Ruby's append mode is derived from C and C's append mode mean "ignore file position".
Which documentation bring you confusing?

http://pubs.opengroup.org/onlinepubs/009695399/functions/fopen.html

Opening a file with append mode (a as the first character in the mode argument) shall cause all 
subsequent writes to the file to be forced to the then current end-of-file, regardless of intervening 
calls to fseek().

#2 Updated by Motohiro KOSAKI almost 4 years ago

  • Status changed from Open to Feedback

#3 Updated by Yusuke Endoh almost 3 years ago

  • Target version set to 2.0.0
  • Status changed from Feedback to Assigned
  • Assignee set to Motohiro KOSAKI

kosaki (Motohiro KOSAKI) wrote:

Which documentation bring you confusing?

IO.new does not say that append mode ignores fseek.
That said, I don't like to make rdoc messy.
How about adding just "See fopen(3) man pages"?

Kosaki-san, may I leave this to you?

Yusuke Endoh mame@tsg.ne.jp

#4 Updated by Motohiro KOSAKI almost 3 years ago

  • Assignee changed from Motohiro KOSAKI to Eric Hodel

Hi Eric,

Currently, rdoc seems failed to understand rb_define_file_const() in io.c and doesn't make correct File::Constant documentation.
Is there any workaround for it?

I don't understand
1) why rb_file_const() is necessary
2) why several constants of File::Constants are defined in io.c, but not file.c
3) why rdoc has a special knowledge for rb_curses_define_const and doesn't have for rb_define_file_const()

#5 Updated by Eric Hodel almost 3 years ago

I will add support for rb_file_const() to RDoc.

#6 Updated by Eric Hodel almost 3 years ago

  • Category set to lib
  • Status changed from Assigned to Closed

#7 Updated by Motohiro KOSAKI almost 3 years ago

Hi drbrain,

Your patch looks don't work. Attached patch created following result.

= File::Constants

(from ruby core)

foo bar.

= Constants:

LOCK_SH:
shared lock

I.e.
- rb_define_const() in file.c work correctly.
- rb_file_const() in file.c doesn't work.
- both rb_define_const() and rb_file_const() in io.c doesn't work.

Can you please tell me how to write a correct annotation, please?

Thanks!

#8 Updated by Zachary Scott almost 3 years ago

=begin
For documenting a constant by (({rb_define_const})), I think you need to use Document-const directive in rdoc.

http://rdoc.rubyforge.org/RDoc/Parser/C.html

((edit s/defined/define))
=end

#9 Updated by Eric Hodel almost 3 years ago

=begin
I haven't yet imported the fix into ruby trunk, I am cleaning up the remaining bugs first. I hope to have it committed this coming weekend. I'll leave this ticket open until then.

Since rb_file_const is special, the comment should look like:

/*
* Document-const: LOCK_SH
*
* Creates a shared lock.
*
* Multiple processes may hold the lock for a file at the same time
*/

I didn't teach RDoc how to look for the comment above the constant properly.
=end

#10 Updated by Motohiro KOSAKI almost 3 years ago

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

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


  • io.c (Init_IO): removed all rb_file_const() into file.c.
  • file.c (Init_File): replace with rb_file_const() with rb_define_const() because RDoc don't care rb_file_const. [Bug #5530]

#11 Updated by Motohiro KOSAKI almost 3 years ago

Finally, I've used rb_define_const() directly and solved the issue.

Thank you!

#12 Updated by Zachary Scott almost 3 years ago

Kosaki-san, it might be good to backport this change so the File::Constants documentation makes it into 1.9.3

#13 Updated by Zachary Scott almost 3 years ago

In anticipation of drbrain's change in rdoc, i've added r37744, see #7365.

This should be reverted and treated similarly to kosaki-san's change in r37746

#14 Updated by Zachary Scott almost 3 years ago

I've added a patch for File::NULL like kosaki-san's patch in r37746

#15 Updated by Zachary Scott almost 3 years ago

  • Status changed from Closed to Assigned
  • Assignee changed from Eric Hodel to Motohiro KOSAKI
  • % Done changed from 100 to 90

#16 Updated by Motohiro KOSAKI almost 3 years ago

Hi Zachary,

thanks. I've commited your patch at r37774 too.

#17 Updated by Motohiro KOSAKI almost 3 years ago

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

Also available in: Atom PDF