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.

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

Actually allow returning null from StaplerProxy #149

Merged
merged 2 commits into from Oct 11, 2018

Conversation

Projects
None yet
2 participants
@daniel-beck
Member

daniel-beck commented Oct 10, 2018

It seems Stapler documents returning null from getTarget() to backtrack (or abort dispatching), but in reality, it doesn't work that way: It's the same as returning this.

I noticed this bug when trying to conditionally have an object not handle a request at all (when the user has no permission to Item/Read it) and fall back to Stapler failing to route.

Upstream of jenkinsci/jenkins#3690

@daniel-beck

This comment has been minimized.

Member

daniel-beck commented Oct 10, 2018

c2ebd74 (plus a local versions:set) deployed as 1.255-null-1-SNAPSHOT.

daniel-beck added a commit to daniel-beck/jenkins that referenced this pull request Oct 10, 2018

@daniel-beck daniel-beck referenced this pull request Oct 10, 2018

Merged

Adapt to stapler/stapler#149 #3690

2 of 2 tasks complete

@daniel-beck daniel-beck requested a review from jglick Oct 10, 2018

@jglick

jglick approved these changes Oct 11, 2018

@@ -697,9 +697,11 @@ boolean tryInvoke(RequestImpl req, ResponseImpl rsp, Object node ) throws IOExce
else

This comment has been minimized.

@jglick

jglick Oct 11, 2018

Member

BTW the Function.renderResponse behavior appears to be undocumented.

@jglick jglick merged commit c2ebd74 into stapler:master Oct 11, 2018

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details

jglick added a commit that referenced this pull request Oct 11, 2018

jglick added a commit to jenkinsci/jenkins that referenced this pull request Oct 11, 2018

olivergondza added a commit to jenkinsci/jenkins that referenced this pull request Oct 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment