Solution to "string iterators incompatible" in debug mode MSVC

[idleman]

Hi,

When I tried to run a basic example:

http::client client;
http::client::request request("http://www.google.com");
request << header("Connection", "close");
http::client::response response = client.get(request);
std::cout << body(response) << std::endl;

I got an assert error: "string iterators incompatible" as several
others have already got before me (MSVC), but I did some research. The
error occurs when when this code is invoked:

string_type query() const {
const_range_type range = query_range();
return string_type(boost::begin(range), boost::end(range));
}
In file "boost/network/uri/uri.hpp" on line 156.

The problem occurs because the method query_range returns a default
constructed const_range_type sometimes:

const_range_type query_range() const {
return uri_parts_.query ? uri_parts_.query.get() :
const_range_type(); //last line is illegal if
//any attempt to compare the iterator are made with the return value,
the iterator is singular
}

Defined in same file in line 119.

It is a problem because the const_range_type is a typedef of:
boost::iterator_range<std::string::const_iterator>
and in the documentation does it say:

"Recall that many default constructed iterators are singular and hence
can only be assigned, but not compared or incremented or anything."
Source: http://www.boost.org/doc/libs/1_49_0/libs/range/doc/html/range/reference/utilities/iterator_range.html

In debug does apparently std::string add some asserts which check if
these iterators are "invalid", which them are if default constructed.
They must be assigned later before any check or increment.

What are your thoughts?

Thanks in advance and for a great attempt to create a simple network
library.

[idleman]

Just to be clear, so everyone know, this can be very simple solved just adding an "if" in every path/query/.. method, like this:

    string_type scheme() const {
        if(!uri_parts_.scheme) {
            return string_type();
        }
        const_range_type range = scheme_range();

        return string_type(boost::begin(range), boost::end(range));
    }

    string_type user_info() const {
        if(!uri_parts_.hier_part.user_info) {
            return string_type();
        }
        const_range_type range = user_info_range();

        return string_type(boost::begin(range), boost::end(range));
    }

    string_type host() const {
        if(!uri_parts_.hier_part.host) {
            return string_type();
        }
        const_range_type range = host_range();

        return string_type(boost::begin(range), boost::end(range));
    }

    string_type port() const {
        if(!uri_parts_.hier_part.port) {
            return string_type();
        }
        const_range_type range = port_range();

        return string_type(boost::begin(range), boost::end(range));
    }

    string_type path() const {
        if(!uri_parts_.hier_part.path) {
            return string_type();
        }
        const_range_type range = path_range();

        return string_type(boost::begin(range), boost::end(range));
    }

    string_type query() const {
        if(!this->uri_parts_.query) {
            return string_type();

        }
        const_range_type range = query_range();
        return string_type(boost::begin(range), boost::end(range));
    }

    string_type fragment() const {
        if(!this->uri_parts_.fragment) {
            return string_type();
        }
        const_range_type range = fragment_range();

        return string_type(boost::begin(range), boost::end(range));
    }

But I doubt this is the only place in the code base you have this. I also don´t know if you have left it with purpose, know the owner of the this project is pretty smart ;-) Anyway, the code above solves every assert error in Windows 7 Win32/x64 with Visual Studio, to run the example:

http::client client;
http::client::request request("http://www.google.com");
request << header("Connection", "close");
http::client::response response = client.get(request);
std::cout << body(response) << std::endl;

Without any problems.

PS. If someone don´t understand me, please let me know, so will I do my best to explain better.

Best regards.

[Dean Michael Berris]

On Mon, Apr 2, 2012 at 9:14 AM, idleman <evo...@gmail.com> wrote:
> Just to be clear, so everyone know, this can be very simple solved just
> adding an "if" in every path/query/.. method, like this:
>

Cool, thanks for the detailed explanation!

Can you prepare a pull request to the official repository with your changes?

Glyn is currently in charge of the URI implementation but if you can
write tests that manifest the errors and these changes, I'm positive
he'll merge in your changes when he gets around to reviewing them.

You can find out how to do this easily from here:
http://help.github.com/send-pull-requests/

Cheers

--
Dean Michael Berris
Technical Solutions Engineer
Google

[Glyn Matthews]

Hi,

On 1 April 2012 21:07, idleman <evo...@gmail.com> wrote:

Hi,

When I tried to run a basic example:

http::client client;
http::client::request request("http://www.google.com");
request << header("Connection", "close");
http::client::response response = client.get(request);
std::cout << body(response) << std::endl;

I got an assert error: "string iterators incompatible" as several
others have already got before me (MSVC), but I did some research. The
error occurs when when this code is invoked:

string_type query() const {
   const_range_type range = query_range();
   return string_type(boost::begin(range), boost::end(range));
 }
In file "boost/network/uri/uri.hpp" on line 156.

The problem occurs because the method query_range returns a default
constructed const_range_type sometimes:

const_range_type query_range() const {
   return uri_parts_.query ? uri_parts_.query.get() :
const_range_type(); //last line is illegal if
//any attempt to compare the iterator are made with the return value,
the iterator is singular
}

I noticed this bug after the 0.9.3 release and I have already applied a fix on the master branch.  Can you let me know if this works for you?

 

Thanks in advance and for a great attempt to create a simple network
library.

Thanks also for the detailed issue report.

Glyn

[idleman]

Your fix works without problems ;-) Thanks!

Den måndagen den 2:e april 2012 kl. 08:47:38 UTC+2 skrev Glyn Matthews:

Hi,

[David Artz]

I'm seeing this same behavior with the cpp-netlib 0.10.1 version (i.e., boost 1.54, windows 7 64bit, visual studio profesional 10). Same error in Debug build, works fine in Release.

[Glyn Matthews]

Hi David,

On 19 July 2013 19:06, David Artz <dar...@gmail.com> wrote:

I'm seeing this same behavior with the cpp-netlib 0.10.1 version (i.e., boost 1.54, windows 7 64bit, visual studio profesional 10). Same error in Debug build, works fine in Release.

I'm aware of bugs of this nature in the 0.x releases. It means they haven't been tested thorougly on MSVC. These problems don't appear in the 1.x version. So it seems there are 3 options:

1. backport the URI in the 1.x branch to the 0.x branch, removing all C++11 features

2. Maintain 0.x and 1.x versions in parallel

3. Tolerate bugs in 0.x

I think the bug you describe is fundamental (internally, it's actually undefined behaviour, and we can't rely on the fact that it just seems to work on most platforms), therefore 3 is not an option. 1. is impractical, as the 1.x URI relies heavily on new C++11 library features (e.g. std::system_error) and backporting will not be trivial. 2. is therefore the least worst option.

I've added an issue (https://github.com/cpp-netlib/cpp-netlib/issues/275) and I'll address it in the next days/weeks.

Thanks,

Glyn

[Simon Haegler]

I'm aware of bugs of this nature in the 0.x releases. It means they haven't been tested thorougly on MSVC. These problems don't appear in the 1.x version. So it seems there are 3 options:

1. backport the URI in the 1.x branch to the 0.x branch, removing all C++11 features

2. Maintain 0.x and 1.x versions in parallel

3. Tolerate bugs in 0.x

I think the bug you describe is fundamental (internally, it's actually undefined behaviour, and we can't rely on the fact that it just seems to work on most platforms), therefore 3 is not an option. 1. is impractical, as the 1.x URI relies heavily on new C++11 library features (e.g. std::system_error) and backporting will not be trivial. 2. is therefore the least worst option.

I've added an issue (https://github.com/cpp-netlib/cpp-netlib/issues/275) and I'll address it in the next days/weeks.

we just got bitten by the "incompatible string iterators" assertion again with debug builds of cpp-netlib 0.11.2, boost 1.59.0 and VC12. in spirit of option 2, i would like to propose a hack/workaround fix:

https://github.com/mistafunk/cpp-netlib/commit/1ed82c51793016e5dd2381b461f4f1eb62a6ce7f

this is not a proper fix but works around the problem for us. let me know, if thats useful for you and i'll produce a pull request.

my insights into the problem so far: the original fix proposed by idleman does not work for us, because it basically only checks if the boost::optional component containers in "uri detail" contain valid iterator ranges. as the parser always produces valid iterator_range objects this check will always return true. IMHO, the actual problem are the default constructed iterator_range<std::string::iterator> instances used by the parser as "skip" values, i.e. as default values if a component (e.g. host) cannot be found in the input uri string. default constructed string iterators are "singular" and therefore also the iterator_range are singular. boost says in [1] that singular iterator_ranges must not be compared (undefined behavior), but this exactly happens if the boolean operator is called on the iterator_range and this is where the assertion is triggered. a proper fix could probably use an iterator range on a sentinel empty string, but this is a bit tricky because it has to be constexpr.

best,

simon

[1] http://www.boost.org/doc/libs/1_59_0/libs/range/doc/html/range/reference/utilities/iterator_range.html

[Dean Michael Berris]

I'd be happy to review a pull request, and would appreciate it very much.

Thanks in advance Simon!

--
You received this message because you are subscribed to the Google Groups "The C++ Network Library" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cpp-netlib+...@googlegroups.com.
To post to this group, send email to cpp-n...@googlegroups.com.
Visit this group at http://groups.google.com/group/cpp-netlib.
For more options, visit https://groups.google.com/d/optout.

[Simon Haegler]

On Wednesday, 14 October 2015 02:22:36 UTC+2, Dean Michael Berris wrote:

I'd be happy to review a pull request, and would appreciate it very much.

here's the pull request:

https://github.com/cpp-netlib/cpp-netlib/pull/565

did you already consider how to properly fix this problem? strictly speaking, the 0.11 branch is currently relying on undefined (compiler/stl dependent) behavior when uri::host(), uri::path(), etc are called on URIs without these components...

best,

simon

[Dean Michael Berris]

Like Glyn mentioned, we have options here.

Personally, I like the idea of just using C++11 features in the 0.11-devel branch anyway, and having a 0.12-devel branch cut to just required C++11 features as a minimum, when we're ready to do another release.

I think Glyn has fixed a lot of these issues in the cpp-netlib/uri implementation, and that we should just use that implementation moving forward.

--

[Simon Haegler]

On Friday, 16 October 2015 02:12:29 UTC+2, Dean Michael Berris wrote:

Like Glyn mentioned, we have options here.

Personally, I like the idea of just using C++11 features in the 0.11-devel branch anyway, and having a 0.12-devel branch cut to just required C++11 features as a minimum, when we're ready to do another release.

I think Glyn has fixed a lot of these issues in the cpp-netlib/uri implementation, and that we should just use that implementation moving forward.

looking at the uri submodule in master, the default values (parser skip values) for missing uri components are still default constructed iterator ranges (i.e. boost::iterator_range<iterator>()), so the undefined behavior when calling range() is still there.

having said that, we recently noticed that there is a C++14 switch in libc++ (the default stl implementation of apple clang) that explicitely initializes default constructed iterators to nullptr and all problems in cpp-netlib (at least on clang) vanish.

best,

simon