It's Time to Play SPOT THE BUG!!

*cheer*

All right, ladies and gentlemen.  I'm your host with the most, BuggyFixius.  *cheer and clappings*

Today we have an exciting web bug for you.  Let's see if you can... *altogether now* SPOT THE BUG!!

We have with us today, an ASP.NET website project *clappings*.  The project contain a helper class like so:

   1:  using System.Web;
   2:   
   3:  public class WebHelper
   4:  {
   5:      const string PARAM_USER_ID = "u";
   6:   
   7:      static HttpRequest request;
   8:   
   9:      static WebHelper()
  10:      {
  11:          request = HttpContext.Current.Request;
  12:      }
  13:   
  14:      public static string GetUserId()
  15:      {
  16:          return getRequest(PARAM_USER_ID);
  17:      }
  18:   
  19:      static string getRequest(string key)
  20:      {
  21:          string value = request[key];
  22:          return string.IsNullOrEmpty(value) ? string.Empty : value;
  23:      }
  24:  }

*Ooo, Aaaa and some gasps*

It also has a Default.aspx file containing a plain Label control

   1:  <%@ Page Language="C#" AutoEventWireup="true"  CodeFile="Default.aspx.cs" Inherits="_Default" %>
   2:   
   3:  <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
   4:   
   5:  <html xmlns="http://www.w3.org/1999/xhtml" >
   6:  <head runat="server">
   7:      <title>Untitled Page</title>
   8:  </head>
   9:  <body>
  10:      <form id="form1" runat="server">
  11:      <div>
  12:          <asp:Label ID="Label1" runat="server" />
  13:      </div>
  14:      </form>
  15:  </body>
  16:  </html>

and the following Page Load event handler:

   1:  using System;
   2:  using System.Web.UI.WebControls;
   3:   
   4:  public partial class _Default : System.Web.UI.Page 
   5:  {
   6:      protected void Page_Load(object sender, EventArgs e)
   7:      {
   8:          Label1.Text = WebHelper.GetUserId();
   9:      }
  10:  }

*Some more Ooo, Aaaa and some more gasps*

Contestants... it's time to *altogether now* SPOT THE BUG!!

 Hints: open Default.aspx in the browser 2 or more times given a different u querystring.  For example: http://localhost:34939/Default.aspx?u=3959 and then replace the url with http://localhost:34939/Default.aspx?u=4000.

Share this post: | | | |
Published Friday, April 25, 2008 5:32 PM by Jimmy Chandra
Filed under: ,

Comments

# re: It's Time to Play SPOT THE BUG!!

Sunday, April 27, 2008 7:37 AM by irwansyah

# re: It's Time to Play SPOT THE BUG!!

Not quite what I was looking for, but that's part of it :).

I'm more interested in how the HttpContext.Current.Request work.

Monday, April 28, 2008 10:41 AM by Jimmy Chandra

# re: It's Time to Play SPOT THE BUG!!

Root problemnya tuh salah design. Tlebih lg pnggunaan static constructor. Tp ini jg reveal kl stiap req aspnet akan sll bt instan httprequest. Tp msti dtelusuri lbih ljt krn aspnet mggunakan thread pool. Yg hrs diingat, jgn prnah gnakan static var dlm platform multithread kyk aspnet

Monday, April 28, 2008 7:50 PM by Irwansyah

# re: It's Time to Play SPOT THE BUG!!

I'll have to disagree with that last comment.  Generalizing that static variable is bad for any kind of usage in a multithreading environment like ASP.NET is not necessarily true, imo.  I'm sure I can find a place or two where static can be proven useful in these situations.

Like I said previously, the problem really lies in the assumption on how HttpContext.Current.Request[...] work.  The assumption was Request[...] will actually do the real querying to get the value, but instead, how it's internally work is that when the Current.Request is created, all the values from querystrings, forms, servervariables have already been cached inside Request's internal fields, and thus the bug. Unfortunately, using static just compound the problem, but it's not the root of the problem.

Thursday, May 01, 2008 11:01 PM by Jimmy Chandra

# re: It's Time to Play SPOT THE BUG!!

Kalo dari hasil investigasi gw (ciee kayak detektif aja), itu static var kan di initnya lewat static constructor. Nah coba deh lo debug, pasti static contructornya cuma dijalanin 1x doang, tetapi untuk request2 selanjutnya enggak. So, ini kenapa gw bilang root problemnya tuh di kesalahan desain.

Untuk HTTPRequest, gw malah berpendapat,  seperti gw bilang di comment gw sebelumnya, selalu dibuat instance HTTPRequest baru untuk setiap request yang masuk. Ini sebabnya HTTPRequest yg udah dihold sama static var ga berisi updated values dari request ke-(n+1).

Untuk static var, menurut gw memang berbahaya kalo ada race condition. Coba deh bikin 1 static var yg dibaca n diisi trus kasih request berkali2 ke page, misal variabel yang tipenya sqlconnection, gw tau ini karena pernah ngalamin bug karena race condition di asp.net.

Friday, May 02, 2008 3:43 PM by irwansyah

Leave a Comment

(required) 
(required) 
(optional)
(required) 

Enter the numbers above: