diff --git a/core/actor/src/main/scala/net/liftweb/actor/LAFuture.scala b/core/actor/src/main/scala/net/liftweb/actor/LAFuture.scala index 0d3fa5713..ff7ce812f 100644 --- a/core/actor/src/main/scala/net/liftweb/actor/LAFuture.scala +++ b/core/actor/src/main/scala/net/liftweb/actor/LAFuture.scala @@ -386,8 +386,8 @@ object LAFuture { val sync = new Object val len = futures.length val accumulator = new ArrayBuffer[Box[T]](len) - // pad array so inserts at random places are possible - for (i <- 0 to len) { accumulator.insert(i, Empty) } + // pad array so values can be placed at their input index + for (i <- 0 until len) { accumulator.insert(i, Empty) } var gotCnt = 0 futures.toList.zipWithIndex.foreach { @@ -435,7 +435,7 @@ object LAFuture { def collect[T](future: LAFuture[T]*): LAFuture[List[T]] = { collect[T, List[T]]( onFutureSucceeded = { (value, result, values, index) => - values.insert(index, Full(value)) + values.update(index, Full(value)) }, onFutureFailed = { (valueBox, result, values, index) => result.fail(valueBox) }, onAllFuturesCompleted = { (result, values) => result.satisfy(values.toList.flatten) }, @@ -455,7 +455,7 @@ object LAFuture { onFutureSucceeded = { (value, result, values, index) => value match { case Full(realValue) => - values.insert(index, Full(Full(realValue))) + values.update(index, Full(Full(realValue))) case other: EmptyBox => result.satisfy(other) } diff --git a/core/actor/src/test/scala/net/liftweb/actor/LatentCollectBugSpec.scala b/core/actor/src/test/scala/net/liftweb/actor/LatentCollectBugSpec.scala new file mode 100644 index 000000000..d4379a9d0 --- /dev/null +++ b/core/actor/src/test/scala/net/liftweb/actor/LatentCollectBugSpec.scala @@ -0,0 +1,57 @@ +/* + * Copyright 2009-2026 Lift Committers and Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.liftweb +package actor + +import org.specs2.mutable.Specification + +import common._ + +/** + * Validates Bug 3: LAFuture.collect over-allocates its accumulator + * (`for (i <- 0 to len)` at LAFuture.scala:390 creates len+1 slots) and uses + * `insert` rather than `update` at the success site, so collected values are + * ordered by completion order rather than by input order. + * + * This example is expected to FAIL while the bug is present. + */ +class LatentCollectBugSpec extends Specification { + "LAFuture.collect Specification".title + + // A scheduler that runs callbacks inline on the calling thread, so that the + // order of satisfy() calls deterministically drives collect()'s accumulation. + private val inline = new LAScheduler { + def execute(f: () => Unit): Unit = f() + } + + "LAFuture.collect" should { + "preserve input order regardless of completion order" in { + val f1 = new LAFuture[String](inline) + val f2 = new LAFuture[String](inline) + val f3 = new LAFuture[String](inline) + + val collected = LAFuture.collect(f1, f2, f3) + + // Complete out of order: middle, then first, then last. + f2.satisfy("b") + f1.satisfy("a") + f3.satisfy("c") + + collected.get(5000L) must beEqualTo(Full(List("a", "b", "c"))) + } + } +} diff --git a/core/util/src/main/scala/net/liftweb/util/CurrencyZone.scala b/core/util/src/main/scala/net/liftweb/util/CurrencyZone.scala index ade97de5d..bd1ae6f71 100644 --- a/core/util/src/main/scala/net/liftweb/util/CurrencyZone.scala +++ b/core/util/src/main/scala/net/liftweb/util/CurrencyZone.scala @@ -142,9 +142,20 @@ abstract class CurrencyZone { def get(numberOfFractionDigits: Int): String = { val nf = NumberFormat.getNumberInstance(_locale) val df = nf.asInstanceOf[DecimalFormat] - val groupingSeparator = df.getDecimalFormatSymbols.getGroupingSeparator - - format("", numberOfFractionDigits).replaceAll(groupingSeparator.toString+"", "") + val symbols = df.getDecimalFormatSymbols + val groupingSeparator = symbols.getGroupingSeparator + val decimalSeparator = symbols.getDecimalSeparator + + // Strip the grouping separator (as a literal, not a regex), + // normalise the locale-specific decimal separator to '.', and drop + // any remaining non-numeric characters (e.g. the non-breaking space + // German currency formatting leaves behind), so the result is always + // a valid BigDecimal string -- even for locales such as + // Locale.GERMANY where the separators are '.' and ','. + format("", numberOfFractionDigits) + .replace(groupingSeparator.toString, "") + .replace(decimalSeparator.toString, ".") + .replaceAll("[^0-9.+-]", "") } } diff --git a/core/util/src/main/scala/net/liftweb/util/SecurityHelpers.scala b/core/util/src/main/scala/net/liftweb/util/SecurityHelpers.scala index 32d15a634..8e19f68c6 100644 --- a/core/util/src/main/scala/net/liftweb/util/SecurityHelpers.scala +++ b/core/util/src/main/scala/net/liftweb/util/SecurityHelpers.scala @@ -45,11 +45,19 @@ trait SecurityHelpers { private def withRandom[T](f: SecureRandom => T): T = _random.synchronized(f(_random)) + /** + * Reduce a (possibly negative) value into the range `[0, mod)`. Unlike + * `math.abs(value) % mod`, this is safe when `value` is `MinValue`, where + * `math.abs` overflows back to a negative number. + */ + private[util] def nonNegativeMod(value: Long, mod: Long): Long = math.floorMod(value, mod) + private[util] def nonNegativeMod(value: Int, mod: Int): Int = math.floorMod(value, mod) + /** return a random Long modulo a number */ - def randomLong(mod: Long): Long = withRandom(random => math.abs(random.nextLong) % mod) + def randomLong(mod: Long): Long = withRandom(random => nonNegativeMod(random.nextLong, mod)) /** return a random int modulo a number */ - def randomInt(mod: Int): Int = withRandom(random => math.abs(random.nextInt) % mod) + def randomInt(mod: Int): Int = withRandom(random => nonNegativeMod(random.nextInt, mod)) /** * return true only 'percent' times when asked repeatedly. diff --git a/core/util/src/main/scala/net/liftweb/util/StringHelpers.scala b/core/util/src/main/scala/net/liftweb/util/StringHelpers.scala index b58fb28a8..50dda3d19 100644 --- a/core/util/src/main/scala/net/liftweb/util/StringHelpers.scala +++ b/core/util/src/main/scala/net/liftweb/util/StringHelpers.scala @@ -52,9 +52,12 @@ trait StringHelpers { * The result is a Map[String, String] */ def splitNameValuePairs(props: String): Map[String, String] = { - val list = props.split(",").toList.map(in => { - val pair = in.roboSplit("=") - (pair(0), unquote(pair(1))) + val list = props.split(",").toList.flatMap(in => { + in.roboSplit("=") match { + case name :: value :: _ => Some((name, unquote(value))) + case name :: Nil if name.nonEmpty => Some((name, "")) + case _ => None + } }) val map: Map[String, String] = Map.empty diff --git a/core/util/src/main/scala/net/liftweb/util/TimeHelpers.scala b/core/util/src/main/scala/net/liftweb/util/TimeHelpers.scala index f4dd4e1b5..d2beada62 100644 --- a/core/util/src/main/scala/net/liftweb/util/TimeHelpers.scala +++ b/core/util/src/main/scala/net/liftweb/util/TimeHelpers.scala @@ -261,7 +261,7 @@ trait TimeHelpers { self: ControlHelpers => */ object TimeSpan { /** time units and values used when converting a total number of millis to those units (see the format function) */ - val scales = List((1000L, "milli"), (60L, "second"), (60L, "minute"), (24L, "hour"), (7L, "day"), (10000L, "week")) + val scales = List((1000L, "milli"), (60L, "second"), (60L, "minute"), (24L, "hour"), (7L, "day"), (Long.MaxValue, "week")) /** explicit constructor for a TimeSpan */ def apply(in: Long) = new TimeSpan(in) diff --git a/core/util/src/test/scala/net/liftweb/util/LatentBugsSpec.scala b/core/util/src/test/scala/net/liftweb/util/LatentBugsSpec.scala new file mode 100644 index 000000000..a896e343e --- /dev/null +++ b/core/util/src/test/scala/net/liftweb/util/LatentBugsSpec.scala @@ -0,0 +1,77 @@ +/* + * Copyright 2006-2026 Lift Committers and Contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package net.liftweb +package util + +import org.specs2.mutable.Specification + +/** + * Each example below is expected to FAIL while the corresponding latent bug is + * present, and to pass once the bug is fixed. + */ +class LatentBugsSpec extends Specification { + "Latent bug validation".title + + "Bug 1: SecurityHelpers.randomLong / randomInt" should { + // randomLong / randomInt reduce the raw RNG output into [0, mod) via + // Helpers.nonNegativeMod, the exact reduction those methods apply. + // SecureRandom.nextLong can return Long.MinValue, and the old + // `math.abs(value) % mod` overflowed (math.abs(Long.MinValue) is still + // negative), yielding a negative result and violating the documented + // "modulo a number" contract. Exercised deterministically with the + // worst-case RNG output. + "never yield a negative value, even when the RNG returns Long.MinValue" in { + Helpers.nonNegativeMod(Long.MinValue, 7L) must be_>=(0L) + } + "never yield a negative value, even when the RNG returns Int.MinValue" in { + Helpers.nonNegativeMod(Int.MinValue, 7) must be_>=(0) + } + } + + "Bug 2: CurrencyZone.get / round (EU / German locale)" should { + // CurrencyZone.scala:147 uses replaceAll (which takes a *regex*) with the + // locale grouping separator. For EU (Locale.GERMANY) the grouping separator + // is '.', so the regex matches every character and wipes out the number. + "keep the digits of a formatted EU amount instead of erasing them" in { + EU("1234.56").get must beMatching(".*1234.*") + } + "not throw when rounding a EU amount" in { + // round(p) = make(BigDecimal(get(p))); with the bug get(p) == "" and + // BigDecimal("") throws NumberFormatException. + EU("1234.56").round(2) must not(throwA[NumberFormatException]) + } + } + + "Bug 4: TimeSpan.format" should { + import Helpers._ + // TimeHelpers.scala:264 caps the final "week" unit with a divisor of 10000, + // so weeks are reported modulo 10000 and whole 10000-week blocks disappear. + "report all weeks for a duration of exactly 10000 weeks" in { + TimeSpan.format(weeks(10000L)) must beEqualTo("10000 weeks") + } + } + + "Bug 5: StringHelpers.splitNameValuePairs" should { + import StringHelpers._ + // StringHelpers.scala:54-62 reads pair(1) without checking that a value was + // actually present, so a malformed entry throws IndexOutOfBoundsException + // instead of being handled gracefully. + "not throw on an entry that has no '=' value" in { + splitNameValuePairs("foo") must not(throwAn[IndexOutOfBoundsException]) + } + } +}