Bug #5530

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

Added by Joshua J. Drake over 2 years ago. Updated over 1 year ago.

[ruby-core:40573]
Status:Closed
Priority:Normal
Assignee:Motohiro KOSAKI
Category:lib
Target version:2.0.0
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 filemodetest.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) [x8664-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
sethome'
<internal:gem
prelude>:38:in dir'
<internal:gem_prelude>:76:in
setpaths'
<internal:gem
prelude>:47:in path'
<internal:gem_prelude>:286:in
pushallhighestversiongemsonloadpath'
<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 over 1 year ago

  • io.c (InitIO): removed all rbfile_const() into file.c.
  • file.c (InitFile): replace with rbfileconst() with rbdefineconst() because RDoc don't care rbfile_const. [Bug #5530]

History

#1 Updated by Motohiro KOSAKI over 2 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 over 2 years ago

  • Status changed from Open to Feedback

#3 Updated by Yusuke Endoh over 1 year 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 over 1 year ago

  • Assignee changed from Motohiro KOSAKI to Eric Hodel

Hi Eric,

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

I don't understand
1) why rbfileconst() 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 rbcursesdefineconst and doesn't have for rbdefinefileconst()

#5 Updated by Eric Hodel over 1 year ago

I will add support for rbfileconst() to RDoc.

#6 Updated by Eric Hodel over 1 year ago

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

#7 Updated by Motohiro KOSAKI over 1 year 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.
- rbdefineconst() in file.c work correctly.
- rbfileconst() in file.c doesn't work.
- both rbdefineconst() and rbfileconst() in io.c doesn't work.

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

Thanks!

#8 Updated by Zachary Scott over 1 year ago

=begin
For documenting a constant by (({rbdefineconst})), 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 over 1 year 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 rbfileconst 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 over 1 year 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 (InitIO): removed all rbfile_const() into file.c.
  • file.c (InitFile): replace with rbfileconst() with rbdefineconst() because RDoc don't care rbfile_const. [Bug #5530]

#11 Updated by Motohiro KOSAKI over 1 year ago

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

Thank you!

#12 Updated by Zachary Scott over 1 year 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 over 1 year 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 over 1 year ago

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

#15 Updated by Zachary Scott over 1 year 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 over 1 year ago

Hi Zachary,

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

#17 Updated by Motohiro KOSAKI over 1 year ago

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

Also available in: Atom PDF