I got the message.



  • Found this gem in the codebase.

                if (!Page.IsPostBack)
                {
                    if (!String.IsNullOrEmpty(Request.QueryString["message"]))
                    {
                        switch (Request.QueryString["message"])
                        {
                            case "created":
                                lblError.Text = "The cost item has been created.";
                                break;
                            case "approved":
                                lblError.Text = "The cost item has been approved.";
                                break;
                            case "edited":
                                lblError.Text = "Your changes to the cost item have been saved.";
                                break;
                            default:
                                break;
                        }
                    }
    

  • Considered Harmful

    Eh. Less WTFy than just printing out Request.QueryString["message"] and opening up an XSS vulnerability. You don't really need to check if it's null or empty before you switch on it though.



  • Am I missing something here? Please elaborate what's so wrong with this. Other than the fact that it's call lblError when it's clearly not an error message.



  • I didn't explicitly say what the real wtf is hoping that some one would pick up on it.

    The work is being done. Then based on the outcome the message parameter is being added to the querystring and the page is reloaded to display the message. Rather that just setting the text property of the label directly.


  • Considered Harmful

    @theflin said:

    I didn't explicitly say what the real wtf is include the wtf in the post hoping that some one would pick up on psychically divine it.

    There are certainly situations where it is valid to do this.



  • @theflin said:

    I didn't explicitly say what the real wtf is hoping that some one would pick up on it.

    The work is being done. Then based on the outcome the message parameter is being added to the querystring and the page is reloaded to display the message. Rather that just setting the text property of the label directly.

    Okay. So instead of something like this:

    [code]
         string message = doSomeWork(thing);
         switch(message)
              case "edit": 
                   lblText.Text = "You've successfully edited";
                   break;
              //etc
    [/code]
    

    It's something like:

    [code]
              string message = doSomeWork(thing);
              Response.Redirect("pageWeAreAlreadyOn.aspx?message=" + message);
    [/code]
    

    That is silly. Not really a WTF, but silly nonetheless.

  • Trolleybus Mechanic

    @joe.edwards said:

    @theflin said:
    I didn't explicitly say what the real wtf is include the wtf in the post hoping that some one would pick up on psychically divine it.
    There are certainly situations where it is valid to do this.
     

    Example:


    protected sub btnCharge_Click(ByVal sender as Object, ByVal e as EventArgs)

         CreatePaymentTransaction();
         ChargeCustomerCreditCart(5000);    

         ' lk - 2013-07-19:  Don't fucking do this! Do you not know how the .net page lifecycle works? Or ANY piece of the Internet at all?      
         ' lblDone.Text = "You're done, buddy!"  

         Response.Redirect("~/payment.aspx?msg=1")

    End Sub

    Since I don't expect you to psychically divine it, here's the corresponding entry from Fictional Bug Tracker:

    @Bug 57263 said:

    What the goddamn fucking hell?  The page finished loading and I wanted to see my new balance. I pressed F5 to refresh the page, and suddenly my credit card was charged twice? I AM GOING TO SUE YOU ASSHOELS!

    Redirecting to another page (or the same page even) kills off the possibility of refreshing the page and re-posting-- which would be the same thing as clicking the button twice. Unless you've built in a very sophisticated dupe-checker and nonce (which I doubt) this is the easiest solution.


  • Considered Harmful

    Sorry, I didn't notice you were agreeing with me.



  • I concede I was too quick to jump on this one. It was to prevent a repost of the page.


  • Trolleybus Mechanic

     I concede I was too quick to jump on this one. It was to prevent a repost of the page.



  • Re: I got the <script>alert('XSS!')</script>.

    I concede I was too quick to jump on this one. It was to prevent a repost of the page.



  • @Lorne Kates said:

    Redirecting to another page (or the same page even) kills off the possibility of refreshing the page and re-posting-- which would be the same thing as clicking the button twice. Unless you've built in a very sophisticated dupe-checker and nonce (which I doubt) this is the easiest solution.

    Sophisticated? It's incredibly easy. Just set a hidden form field with a unique identifier for the transaction, then check on the backend that it's not been used. Redirecting to a new page is not a fool-proof way to stop double posting. I would still consider it a bug for all but the most trivial of applications, where you don't really care if you get double posts.



  • @morbiuswilters said:

    @Lorne Kates said:
    Redirecting to another page (or the same page even) kills off the possibility of refreshing the page and re-posting-- which would be the same thing as clicking the button twice. Unless you've built in a very sophisticated dupe-checker and nonce (which I doubt) this is the easiest solution.

    Sophisticated? It's incredibly easy. Just set a hidden form field with a unique identifier for the transaction, then check on the backend that it's not been used. Redirecting to a new page is not a fool-proof way to stop double posting. I would still consider it a bug for all but the most trivial of applications, where you don't really care if you get double posts.

    @Bug 57264 said:

    FUCKING HELL WHY DOES IT CHARGE MY CREDIT CARD TWICE IF I PUSH BACKSPACE ONCE


  • Considered Harmful

    @morbiuswilters said:

    @Lorne Kates said:
    Redirecting to another page (or the same page even) kills off the possibility of refreshing the page and re-posting-- which would be the same thing as clicking the button twice. Unless you've built in a very sophisticated dupe-checker and nonce (which I doubt) this is the easiest solution.

    Sophisticated? It's incredibly easy. Just set a hidden form field with a unique identifier for the transaction, then check on the backend that it's not been used. Redirecting to a new page is not a fool-proof way to stop double posting. I would still consider it a bug for all but the most trivial of applications, where you don't really care if you get double posts.


    Even if you do dupe checking, you still want to redirect to avoid the browser warning.



  • @joe.edwards said:

    @morbiuswilters said:
    @Lorne Kates said:
    Redirecting to another page (or the same page even) kills off the possibility of refreshing the page and re-posting-- which would be the same thing as clicking the button twice. Unless you've built in a very sophisticated dupe-checker and nonce (which I doubt) this is the easiest solution.

    Sophisticated? It's incredibly easy. Just set a hidden form field with a unique identifier for the transaction, then check on the backend that it's not been used. Redirecting to a new page is not a fool-proof way to stop double posting. I would still consider it a bug for all but the most trivial of applications, where you don't really care if you get double posts.


    Even if you do dupe checking, you still want to redirect to avoid the browser warning.

    I guess. The browser message seems really minor to me, and I don't think I've done a regular form in years, so..



  • @Lorne Kates said:

      ' lk - 2013-07-19:  Don't fucking do this! Do you not know how the .net page lifecycle works? Or ANY piece of the Internet at all?      
         ' lblDone.Text = "You're done, buddy!"  

         Response.Redirect("~/payment.aspx?msg=1")

     

    WTF is that? I tought .Net did evolve further than just echoing everything you write into the socket. Is MS still at the PHP times?

     



  • @Bug 57264 said:

    FUCKING HELL WHY DOES IT CHARGE MY CREDIT CARD TWICE IF I PUSH BACKSPACE ONCE
     

    It doesn't. If you push backspace, you'll get into the page where you clicked a button to confirm the payment.

    But that's just nitpicking. Yes, the transaction should expire after the payment is made, and yes, it's an incredibly easy thing to do.



  • @Mcoder said:

    @Bug 57264 said:

    FUCKING HELL WHY DOES IT CHARGE MY CREDIT CARD TWICE IF I PUSH BACKSPACE ONCE
     

    It doesn't. If you push backspace, you'll get into the page where you clicked a button to confirm the payment.

    You missed the joke.

     



  • @morbiuswilters said:

    Sophisticated? It's incredibly easy. Just set a hidden form field with a unique identifier for the transaction, then check on the backend that it's not been used. Redirecting to a new page is not a fool-proof way to stop double posting. I would still consider it a bug for all but the most trivial of applications, where you don't really care if you get double posts.

    Indeed. That didn't stop my predecessors though, they tried everything from disabling the submit button with JS to redirecting both GETs and POSTs and copy-pasting variations on the same half-broken "context" check in each and every from handler.

    After replacing that nonsense with a UUID sent out with each GET, Spring decided that in some edge cases it should send the UUID back in in the POST even when no hidden field was explicitly declared. That was "fixed" by wrapping the UUID string in an object.


Log in to reply