Skip to content

Commit

Permalink
Avoid surprise NPEs on param / sitemap methods
Browse files Browse the repository at this point in the history
It's possible to get a surprise NPE if you try to read the currentValue
of a loc that's not the current loc instance. This is because the Loc
you're trying to read may not have a SiteMap assigned to it, which will
cause explosions in param calculation.

We now guard against this in various high-profile public API methods so
that we minimize the footprint of surprise NPEs.
  • Loading branch information
farmdawgnation committed May 12, 2018
1 parent 47d4e06 commit 52b2e91
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 8 deletions.
28 changes: 21 additions & 7 deletions web/webkit/src/main/scala/net/liftweb/sitemap/Loc.scala
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,16 @@ trait Loc[T] {

def params: List[Loc.LocParam[T]]

def allParams: List[Loc.AnyLocParam] =
(params.asInstanceOf[List[Loc.AnyLocParam]]) :::
parentParams :::
siteMap.globalParams
def allParams: List[Loc.AnyLocParam] = {
if (_menu == null) {
// Params are impossible without a menu
Nil
} else {
(params.asInstanceOf[List[Loc.AnyLocParam]]) :::
parentParams :::
siteMap.globalParams
}
}

private def parentParams: List[Loc.AnyLocParam] =
_menu match {
Expand All @@ -154,7 +160,10 @@ trait Loc[T] {

def hideIfNoKids_? = placeHolder_? || _hideIfNoKids_?

def siteMap: SiteMap = _menu.siteMap
// This implementation is less than ideal, but changing the type would
// have been a breaking API change. This at least avoids this method
// throwing an NPE
def siteMap: SiteMap = Option(_menu).map(_.siteMap).getOrElse(null)

def createDefaultLink: Option[NodeSeq] =
currentValue.flatMap(p => link.createLink(p)).toOption.
Expand Down Expand Up @@ -401,7 +410,13 @@ trait Loc[T] {
)
}

def breadCrumbs: List[Loc[_]] = _menu.breadCrumbs ::: List(this)
def breadCrumbs: List[Loc[_]] = {
if (_menu != null) {
_menu.breadCrumbs ::: List(this)
} else {
List(this)
}
}

def buildKidMenuItems(kids: Seq[Menu]): List[MenuItem] = {
kids.toList.flatMap(_.loc.buildItem(Nil, false, false)) ::: supplementalKidMenuItems
Expand Down Expand Up @@ -513,7 +528,6 @@ object Loc {
init()
}


/**
* Algebraic data type for parameters that modify handling of a Loc
* in a SiteMap
Expand Down
11 changes: 10 additions & 1 deletion web/webkit/src/test/scala/net/liftweb/sitemap/LocSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ object LocSpec extends Specification {
}
}
}

"not throw Exceptions on param methods before SiteMap assignment" in {
val testMenu = Menu.param[Param]("Test", "Test", s => Empty, p => "bacon") / "foo" / "bar" >> Loc.MatchWithoutCurrentValue
val testLoc = testMenu.toLoc

testLoc.allParams must not(throwA[Exception])
testLoc.currentValue must not(throwA[Exception])
testLoc.siteMap must not(throwA[Exception])
testLoc.breadCrumbs must not(throwA[Exception])
}
}
}

0 comments on commit 52b2e91

Please sign in to comment.