Skip to content

Commit

Permalink
escape res.redirect() link
Browse files Browse the repository at this point in the history
  • Loading branch information
tj committed Aug 3, 2012
1 parent 4892305 commit bac0c64
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/response.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -621,7 +621,8 @@ res.redirect = function(url){
}, },


html: function(){ html: function(){
body = '<p>' + statusCodes[status] + '. Redirecting to <a href="' + url + '">' + url + '</a></p>'; var u = utils.escape(url);
body = '<p>' + statusCodes[status] + '. Redirecting to <a href="' + u + '">' + u + '</a></p>';
}, },


default: function(){ default: function(){
Expand Down
17 changes: 17 additions & 0 deletions test/res.redirect.js
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -232,6 +232,23 @@ describe('res', function(){
done(); done();
}) })
}) })

it('should escape the url', function(done){
var app = express();

app.use(function(req, res){
res.redirect('<lame>');
});

request(app)
.get('/')
.set('Host', 'http://example.com')
.set('Accept', 'text/html')
.end(function(err, res){
res.text.should.equal('<p>Moved Temporarily. Redirecting to <a href="//http://example.com/&lt;lame&gt;">//http://example.com/&lt;lame&gt;</a></p>');
done();
})
})
}) })


describe('when accepting text', function(){ describe('when accepting text', function(){
Expand Down

1 comment on commit bac0c64

@jonchurch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where escaping html in redirects first came into express.

It is unclear if the intent was to prevent XSS-like attacks or to protect the validity of the HTML we render on this redirect page.

Likely it was both.

Please sign in to comment.