Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Modifying a StringIO can overwrite other strings derived from it #5203

Closed
pcarlisle opened this issue May 30, 2018 · 1 comment
Closed

Modifying a StringIO can overwrite other strings derived from it #5203

pcarlisle opened this issue May 30, 2018 · 1 comment
Assignees
@kares
Labels
stdlib
Milestone
JRuby 9.1.18.0

Comments

@pcarlisle
Copy link

pcarlisle commented May 30, 2018 •
edited
Loading

Environment

Linux wait-and-see 4.16.8-1-ARCH #1 SMP PREEMPT Wed May 9 11:25:02 UTC 2018 x86_64 GNU/Linux

jruby 9.2.0.0 (2.5.0) 2018-05-24 81156a8 Java HotSpot(TM) 64-Bit Server VM 25.162-b12 on 1.8.0_162-b12 +jit [linux-x86_64]

I am testing some file rewriting by using a StringIO instance. The original contents get parsed, resulting in various other strings, and then the StringIO is rewritten. I've found that the rewriting will modify the other strings from parsing.

I've managed to reproduce it with this standalone script:

require 'stringio'

a = []

# The interpolation here makes the sharelevel 0 on the argument to StringIO.new,
# which is necessary to trigger the issue
# I'm sure there's a better way but this was how I encountered it
thing = 'kjh'
strio = StringIO.new(<<EOS)
      before = value to keep before
      base = #{thing}
      after = value to keep
EOS

strio.each_line do |line|
  a << line
end

strio.rewind
strio.write('xxxxxxxx')

puts a

Since a is an array of the original lines from each_line this script should print the original string argument to StringIO.new, but instead it includes the output xxxxxxxx overwriting part of the first line.

I've poked around a bit, and it seems like in this case StringIO.getline creates a substring such that the new string is shared but the original doesn't get marked as shared.
@pcarlisle pcarlisle changed the title Modifying a StringIO overwrites other strings derived from it Modifying a StringIO can overwrite other strings derived from it May 30, 2018
@kares kares added this to the JRuby 9.2.1.0 milestone May 31, 2018
@kares kares added the stdlib label May 31, 2018
@kares kares self-assigned this May 31, 2018
@kares kares closed this as completed in b087544 May 31, 2018
pcarlisle added a commit to pcarlisle/puppet that referenced this issue May 31, 2018
@pcarlisle
(PUP-1952) Work around JRuby issue puppetlabs#5203

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
4b10cb9 
By using a literal string with no interpolation in the construction of a
stringio instance we avoid a subtle string sharing bug that was causing
corruption.

jruby/jruby#5203
pcarlisle added a commit to pcarlisle/puppet that referenced this issue Jun 1, 2018
@pcarlisle
(PUP-1952) Work around JRuby issue puppetlabs#5203
c889926 
By using a literal string with no interpolation in the construction of a
stringio instance we avoid a subtle string sharing bug that was causing
corruption.

jruby/jruby#5203
pcarlisle added a commit to pcarlisle/puppet that referenced this issue Jun 1, 2018
@pcarlisle
(PUP-1952) Work around JRuby issue puppetlabs#5203
bbed8af 
By using a literal string with no interpolation in the construction of a
stringio instance we avoid a subtle string sharing bug that was causing
corruption.

jruby/jruby#5203
pcarlisle added a commit to pcarlisle/puppet that referenced this issue Jun 4, 2018
@pcarlisle
(PUP-1952) Work around JRuby issue puppetlabs#5203
9256363 
By using a literal string with no interpolation in the construction of a
stringio instance we avoid a subtle string sharing bug that was causing
corruption.

jruby/jruby#5203
pcarlisle added a commit to pcarlisle/puppet that referenced this issue Jun 5, 2018
@pcarlisle
(PUP-1952) Work around JRuby issue puppetlabs#5203
99cddfa 
By using a literal string with no interpolation in the construction of a
stringio instance we avoid a subtle string sharing bug that was causing
corruption.

jruby/jruby#5203
pcarlisle added a commit to pcarlisle/puppet that referenced this issue Jun 5, 2018
@pcarlisle
(PUP-1952) Work around JRuby issue puppetlabs#5203
759bbec 
By using a literal string with no interpolation in the construction of a
stringio instance we avoid a subtle string sharing bug that was causing
corruption.

jruby/jruby#5203
kares added a commit that referenced this issue Jun 12, 2018
@kares
[fix] potential string sharing as each_line returns shared buffer
e485ef3 
... later modification to StringIO#string might affect the line

resolves GH-5203
@kares kares modified the milestones: JRuby 9.2.1.0, JRuby 9.1.18.0 Jun 12, 2018
@aryeh-looker
Copy link
Contributor

This bug breaks the S3 AWS SDK gem when using the multipart upload API to an invalid region aws/aws-sdk-ruby#1843. I have verified that b087544 fixes the issue by:

  • running the repro in the S3 ticket under master — it succeeds
  • running the repro in the S3 ticket under master with a revert of b087544 — it fails

Looking forward to this being in 9.1.18.0 and thanks for your fix @kares!

@aryeh-looker aryeh-looker mentioned this issue Jul 29, 2018
Closed
aryeh-looker pushed a commit to aryeh-looker/jruby that referenced this issue Jul 29, 2018
Add Another Regresion Test for jrubyGH-5203
f092873
@aryeh-looker aryeh-looker mentioned this issue Jul 29, 2018
Merged
enebo added a commit that referenced this issue Sep 19, 2018
@enebo
Merge pull request #5265 from aryeh-looker/aryeh/add-regression-test-…
4f75b6c 
…gh-5203

Add Complimentary Regression Test for GH-5203
ccaviness pushed a commit to ccaviness/puppet that referenced this issue Sep 19, 2018
@pcarlisle @ccaviness
(PUP-1952) Work around JRuby issue puppetlabs#5203
5ff4bc4 
By using a literal string with no interpolation in the construction of a
stringio instance we avoid a subtle string sharing bug that was causing
corruption.

jruby/jruby#5203
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kares kares

Labels
stdlib
Projects
None yet
Milestone
JRuby 9.1.18.0
Development

No branches or pull requests
3 participants
@kares @pcarlisle @aryeh-looker