Clean up style for redirects

Author: driverdan

lib/XMLHttpRequest.js

@@ -176,7 +176,7 @@ exports.XMLHttpRequest = function() {
    */
   this.setDisableHeaderCheck = function(state) {
     disableHeaderCheck = state;
-  }
+  };
 
   /**
    * Sets a header for the request.
@@ -249,7 +249,7 @@ exports.XMLHttpRequest = function() {
     }
 
     return "";
-  }
+  };
 
   /**
    * Sends the request to the server.
@@ -378,29 +378,35 @@ exports.XMLHttpRequest = function() {
       // As per spec, this is called here for historical reasons.
       self.dispatchEvent("readystatechange");
 
-      // Create the request
-      
+      // Handler for the response
       function responseHandler(resp) {
-
+        // Set response var to the response we got back
+        // This is so it remains accessable outside this scope
         response = resp;
-
-      	if(response.statusCode === 302 || response.statusCode === 307 || response.statusCode === 303){
-          settings.url = response.headers['location']
+        // Check for redirect
+        // @TODO Prevent looped redirects
+        if (response.statusCode === 302 || response.statusCode === 303 || response.statusCode === 307) {
+          // Change URL to the redirect location
+          settings.url = response.headers.location;
           var url = Url.parse(settings.url);
-          host = url.hostname
+          // Set host var in case it's used later
+          host = url.hostname;
+          // Options for the new request
           var newOptions = {
             hostname: url.hostname,
             port: url.port,
             path: url.path,
             method: response.statusCode === 303 ? 'GET' : settings.method,
             headers: headers
-          }
+          };
 
+          // Issue the new request
           request = doRequest(newOptions, responseHandler).on('error', errorHandler);
-          request.end()
-          return
+          request.end();
+          // @TODO Check if an XHR event needs to be fired here
+          return;
         }
-      
+
         response.setEncoding("utf8");
 
         setState(self.HEADERS_RECEIVED);
@@ -429,10 +435,13 @@ exports.XMLHttpRequest = function() {
           self.handleError(error);
         });
       }
+
+      // Error handler for the request
       function errorHandler(error) {
         self.handleError(error);
       }
 
+      // Create the request
       request = doRequest(options, responseHandler).on('error', errorHandler);
 
       // Node 0.4 and later won't accept empty data. Make sure it's needed.

tests/test-redirect-302.js

@@ -6,11 +6,10 @@ var sys = require("util")
 
 // Test server
 var server = http.createServer(function (req, res) {
-
-  if(req.url === '/redirectingResource'){
-    res.writeHead(302, {'Location': 'http://localhost:8000/'})
-    res.end()
-    return
+  if (req.url === '/redirectingResource') {
+    res.writeHead(302, {'Location': 'http://localhost:8000/'});
+    res.end();
+    return;
   }
 
   var body = "Hello World";
@@ -28,8 +27,8 @@ var server = http.createServer(function (req, res) {
 
 xhr.onreadystatechange = function() {
   if (this.readyState == 4) {
-  	assert.equal(xhr.getRequestHeader('Location'), '')
-  	assert.equal(xhr.responseText, "Hello World")
+    assert.equal(xhr.getRequestHeader('Location'), '');
+    assert.equal(xhr.responseText, "Hello World");
     sys.puts("done");
   }
 };

tests/test-redirect-303.js

@@ -6,11 +6,10 @@ var sys = require("util")
 
 // Test server
 var server = http.createServer(function (req, res) {
-
-  if(req.url === '/redirectingResource'){
-    res.writeHead(303, {'Location': 'http://localhost:8000/'})
-    res.end()
-    return
+  if (req.url === '/redirectingResource') {
+    res.writeHead(303, {'Location': 'http://localhost:8000/'});
+    res.end();
+    return;
   }
 
   var body = "Hello World";
@@ -28,8 +27,8 @@ var server = http.createServer(function (req, res) {
 
 xhr.onreadystatechange = function() {
   if (this.readyState == 4) {
-  	assert.equal(xhr.getRequestHeader('Location'), '')
-  	assert.equal(xhr.responseText, "Hello World")
+    assert.equal(xhr.getRequestHeader('Location'), '');
+    assert.equal(xhr.responseText, "Hello World");
     sys.puts("done");
   }
 };

tests/test-redirect-307.js

@@ -6,14 +6,13 @@ var sys = require("util")
 
 // Test server
 var server = http.createServer(function (req, res) {
-
-  if(req.url === '/redirectingResource'){
-    res.writeHead(307, {'Location': 'http://localhost:8000/'})
-    res.end()
-    return
+  if (req.url === '/redirectingResource') {
+    res.writeHead(307, {'Location': 'http://localhost:8000/'});
+    res.end();
+    return;
   }
-  
-  assert.equal(req.method, 'POST')
+
+  assert.equal(req.method, 'POST');
 
   var body = "Hello World";
   res.writeHead(200, {
@@ -30,8 +29,8 @@ var server = http.createServer(function (req, res) {
 
 xhr.onreadystatechange = function() {
   if (this.readyState == 4) {
-  	assert.equal(xhr.getRequestHeader('Location'), '')
-  	assert.equal(xhr.responseText, "Hello World")
+    assert.equal(xhr.getRequestHeader('Location'), '');
+    assert.equal(xhr.responseText, "Hello World");
     sys.puts("done");
   }
 };