From 1753335d345d85e15b6003be85bbc908e7f6b1a1 Mon Sep 17 00:00:00 2001
From: Gustavo Massaccesi <gustavo@oma.org.ar>
Date: Fri, 14 Aug 2015 00:09:53 -0300
Subject: [PATCH] Fix string-replace when the string is mutable

The `from` string argument is converted to a regexp and cached. When `from` is
a mutable string this can cause wrong results in the following calls
to string-replace. So the string is first converted to an immutable string to
be used as the key for the cache.
---
 .../racket-test-core/tests/racket/string.rktl |  7 +++++
 racket/collects/racket/string.rkt             | 29 +++++++++++++++----
 2 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/pkgs/racket-test-core/tests/racket/string.rktl b/pkgs/racket-test-core/tests/racket/string.rktl
index e429144c2d..cf0a959119 100644
--- a/pkgs/racket-test-core/tests/racket/string.rktl
+++ b/pkgs/racket-test-core/tests/racket/string.rktl
@@ -483,4 +483,11 @@
   (test "foo\\1bar" string-replace "foo===bar" #rx"(=+)" "\\1")
   (test "foo\\1bar" string-replace "foo===bar" #px"={3}" "\\1"))
 
+;test that mutable string are not cached incorrectly
+(let ([str (string-copy "_1_")])
+  (test "!!! _2_" string-replace "_1_ _2_" str "!!!") ;add str to the internal cache
+  (string-set! str 1 #\2)
+  (test "_1_ !!!" string-replace "_1_ _2_" str "!!!") ;verify that the new str is used
+)
+
 (report-errs)
diff --git a/racket/collects/racket/string.rkt b/racket/collects/racket/string.rkt
index 7df9f8e839..29e3e57367 100644
--- a/racket/collects/racket/string.rkt
+++ b/racket/collects/racket/string.rkt
@@ -100,17 +100,36 @@
   (string-join (internal-split 'string-normalize-spaces str sep trim? +?)
                space))
 
-(define replace-cache (make-weak-hasheq))
+
+;; Caches for string-replace:
+;; A mutable string weakly holds a immutable copy until it is collected
+;; or modified (and used as a argument of string-replace).
+;; The immutable copy weakly holds the regexp that is used in string-replace.
+;; Using string->immutable-string directly in string-replace is not a useful
+;; because the immutable copy could be immediately collected.
+ 
+(define immutable-cache (make-weak-hasheq))
+(define (string->immutable-string/cache str)
+  (if (immutable? str)
+    str
+    (let ([old (hash-ref immutable-cache str #f)])
+      (if (and old (string=? str old))
+        old
+        (let ([new (string->immutable-string str)])
+          (hash-set! immutable-cache str new)
+          new)))))    
+
+(define replace-cache (make-weak-hash))
 (define (string-replace str from to #:all? [all? #t])
   (unless (string? str) (raise-argument-error 'string-replace "string?" str))
   (unless (string? to)  (raise-argument-error 'string-replace "string?" to))
+  (unless (or (string? from) (regexp? from))
+    (raise-argument-error 'string-replace "(or/c string? regexp?)" from))
   (define from*
     (if (regexp? from)
       from
-      (hash-ref! replace-cache from
-        (λ() (if (string? from)
-               (regexp (regexp-quote from))
-               (raise-argument-error 'string-replace "string?" from))))))
+      (hash-ref! replace-cache (string->immutable-string/cache from)
+        (λ() (regexp (regexp-quote from))))))
   (define to* (regexp-replace-quote to))
   (if all?
     (regexp-replace* from* str to*)