tags:

views:

308

answers:

5
A: 

Looks like you need an else after

if(bodypos==std::string::npos)
{
    HttpError(...);
}

otherwise you are calling substr with bodypos = npos

Jeff
No, because HttpError exits the program. I appreciate that advice, but the main reason I posted here is to find out why the substr is failing on Good strings.
Stanislav Palatnik
@Stanislav Palatnik: The way to never find your error is to have a preconceived notion about what the error is.
Omnifarious
Create a small test program that includes this code, then call it. Set a breakpoint in the debugger and you'll see your mistake as you examine the variables as you step through code.
ux9i
For the record, this answer was posted before the `else` existed, and (obviously) before we knew `HttpError` would exit the function. A valid but now irrelevant answer.
GMan
A: 

You might consider using the (unsigned) type std::string::size_type instead of int.

Why are you casting the result of find to an int here: int(request.find("register"))!=std::string::npos

Matt Curtis
std::string body = request.substr(bodypos); . Now back to me question :)
Stanislav Palatnik
OK I've removed that question from my answer ("Where is the exception happening?") What about my second question: why are you casting to int? find returns an unsigned std::string::size_type
Matt Curtis
Hi Stanislav. Since I posted this answer you've edited your original question so the casts are now gone and you use size_t (not std::string::size_type, but you get the idea). Can you please confirm that you've also edited and tested your code, and the exception still occurs? Cheers.
Matt Curtis
Yes, it still occurs. I even hardcoded a test string and called substr, and it gave the same error.
Stanislav Palatnik
+1  A: 

Are you sure that it's failing on that substr and not on a substr call within the HttpError or StringExplode functions? If you haven't already, you should run this through a debugger so that you can see exactly where it's throwing the exception. Alternatively, you could add a:

std::cout << "calling substr" << std::endl;

line immediately before you call substr, and a similar line immediately afterwards, so that it would look like:

std::cout << "calling substr" << std::endl;
std::string body = request.substr(bodypos);
std::cout << "finished calling substr" << std::endl;

StringExplode(body,"&", "=",keyval);
outRequest = "doing stuff";

If that substr really is throwing the exception, then you'll know because the program will print "calling substr" without a matching "finished calling substr". If it prints the pair of debug messages, though, or none at all, then something else is throwing the exception.

Josh Townzen
+6  A: 

I modified your sample slightly to decrease amount of indentation used.
There are 5 "test cases" and none causes any problem. Could you please provide a sample request to reproduce the problem you're having.

EDIT: Forgot to mention: if this sample as it is (with commented-out bits) doesn't produce that error, your best bet is that you have a bug in your StringExplode function. You could post its source, to get a more helpful advice.

EDIT2: In your StringExplode, change results[tmpKey] = tmpKey.substr(found+1); to results[tmpKey] = tmpResult[i].substr(found+1);. Change int found to size_t found, and remove all of if (found > 0), that will fix your mysterious out_of_range. You were substr-ing a wrong string. Just in case, here's the code with a fix:

void StringExplode(std::string str, std::string objseparator, std::string keyseperator,
                   std::map <std::string, std::string> &results)
{
    size_t found;
    std::vector<std::string> tmpResult;
    found = str.find_first_of(objseparator);
    while(found != std::string::npos)
    {
        tmpResult.push_back(str.substr(0,found));
        str = str.substr(found+1);
        found = str.find_first_of(objseparator);
    }
    if(str.length() > 0)
    {
        tmpResult.push_back(str);
    }

    for(size_t i = 0; i < tmpResult.size(); i++)
    {
        found = tmpResult[i].find_first_of(keyseperator);
        while(found != std::string::npos)
        {
                std::string tmpKey = tmpResult[i].substr(0, found);
                results[tmpKey] = tmpResult[i].substr(found+1);
                found = tmpResult[i].find_first_of(keyseperator, found + results[tmpKey].size());
        }

    }
}

Initial test code:

#include <iostream>
#include <map>
#include <string>

std::string parse(const std::string &request)
{
    std::map<std::string,std::string> keyval;
    std::string outRequest;

    if(request[0] != 'P')
        return outRequest;

    if(request.find("register") == std::string::npos)
        return outRequest;

    //we have a register request
    size_t bodypos = request.find("username");
    if(bodypos==std::string::npos)
    {
        // HttpError(400,"Malformed HTTP POST request. Could not find key username.",request);
        // you said HttpError returns, so here's a return
        return outRequest;
    }

    std::string body = request.substr(bodypos);
    // StringExplode(body,"&", "=",keyval);
    outRequest = "doing stuff";

    return outRequest;
}

int main()
{

    std::string request("P\r\nregister\r\nusername=hello\r\n\r\n");
    std::cout << "[" << parse(request) << "]\n";

    request = "Pregisternusername=hello\r\n\r\n";
    std::cout << "[" << parse(request) << "]\n";

    request = "Pregisternusername=hello";
    std::cout << "[" << parse(request) << "]\n";

    request = "registernusername=hello";
    std::cout << "[" << parse(request) << "]\n";

    request = "";
    std::cout << "[" << parse(request) << "]\n";

    return 0;
}

This outputs, predictably:

[doing stuff]
[doing stuff]
[doing stuff]
[]
[]

Dmitry
I tried hardcoding that string string, and it gives me the same error. Not sure what's going on here :/std::string request2("P\r\nregister\r\nusername=hello\r\n\r\n");std::string body = request2.substr(4);
Stanislav Palatnik
@Stanislav Palatnik, could you please post the source for `StringExplode`? Most likely the bug is there.
Dmitry
@Stanislav Can you try Dmitry's sample program with your compiler, using the same compiler and linker settings you're using for your other program? Maybe you have some problem with your environment.
Matt Curtis
@ Dmitry: I posted the complete code in the original post, but here is just the code for StringExplode : http://pastebin.ca/1795667@ Curtis: Yes, Dmitry's code runs just fine.
Stanislav Palatnik
@Stanislav Palatnik, check my EDIT2 in the answer, that should help.
Dmitry
Yup, the culprit was StringExplode! Thanks alot!
Stanislav Palatnik
nice work Dmitry!
Matt Curtis
And, technically, it should be `::std::string::size_t`, not just `size_t`.
Omnifarious
+1  A: 

One fairly obvious thing wrong with your code:

int k = read(ns, buf, sizeof(buf)-1);
buf[k] = '\0';

You are not checking that read() succeeded - it returns -1 on failure which will cause all sorts of memory corruption problems if it occurs.

Also:

char * buf2 = const_cast<char *>(reply.c_str());
write(ns,buf2,sizeof(buf2));

You are taking the size of the pointer - you want the length of the output string:

write(ns, buf2, reply.size() );

And you should once again test that write succeeded and that it wrote as many bytes as you requested, though this shouldn't directly cause the substr() error.

anon
Thanks for that tip.
Stanislav Palatnik