From 059138a5df5781255f5c8fdefd72fdf20b7c9538 Mon Sep 17 00:00:00 2001 From: Christian Herdtweck Date: Mon, 15 Dec 2014 15:30:51 +0100 Subject: [PATCH] changed logic of icmp package read/write and operator>> / operator<< * read and write are implemented on their own, >> and << call them (not the other way round as before) * read returns not only a bool but a more informative flag * moved operator>> and << from Icmpv4/6packet to base class IcmpPacket since they are identical --- src/icmp/icmppacket.cpp | 39 +++++++++++++++++ src/icmp/icmppacket.h | 19 ++++++++- src/icmp/icmpv4packet.cpp | 105 ++++++++++++++++++++++----------------------- src/icmp/icmpv4packet.h | 12 +---- src/icmp/icmpv6packet.cpp | 96 ++++++++++++++++++++++++----------------- src/icmp/icmpv6packet.h | 12 +---- 6 files changed, 169 insertions(+), 114 deletions(-) diff --git a/src/icmp/icmppacket.cpp b/src/icmp/icmppacket.cpp index 43d272a..50a616d 100644 --- a/src/icmp/icmppacket.cpp +++ b/src/icmp/icmppacket.cpp @@ -19,6 +19,7 @@ */ #include "icmp/icmppacket.h" +#include //----------------------------------------------------------------------------- // IcmpPacket @@ -37,3 +38,41 @@ IcmpPacket::IcmpPacket() IcmpPacket::~IcmpPacket() { } + +/** + * + * + */ +{ + + +} + +/** + * @brief create packet from data in @a is; calls IcmpPacket::read + * + * @param is input stream with hopefully sufficient data + * + * @returns stream with less data; failbit is set if read returns other than ReadReturnCode_OK + */ +std::istream& operator>>( + std::istream &is, + IcmpPacket &packet +) +{ + IcmpPacket::ReadReturnCode return_code = packet.read(is); + if ( return_code != IcmpPacket::ReadReturnCode_OK ) + is.setstate( std::ios::failbit ); + + return is; +} + + +std::ostream& operator<<( + std::ostream& os, + const IcmpPacket& packet +) +{ + packet.write( os ); + return os; +} diff --git a/src/icmp/icmppacket.h b/src/icmp/icmppacket.h index 706eadb..e37686e 100644 --- a/src/icmp/icmppacket.h +++ b/src/icmp/icmppacket.h @@ -43,9 +43,26 @@ public: ) const = 0; virtual void print_destination_unreachable() const = 0; - virtual bool read( std::istream &is ) = 0; + enum ReadReturnCode { + ReadReturnCode_OK = 0, + ReadReturnCode_FAIL, + ReadReturnCode_NOT_ENOUGH_DATA, + ReadReturnCode_BAD_STREAM, + ReadReturnCode_INVALID_SIZE + }; + + virtual ReadReturnCode read( std::istream &is ) = 0; virtual bool write( std::ostream &os ) const = 0; + friend std::istream& operator>>( + std::istream &is, + IcmpPacket &packet + ); + friend std::ostream& operator<<( + std::ostream &os, + const IcmpPacket &packet + ); + protected: IcmpPacket(); virtual ~IcmpPacket(); diff --git a/src/icmp/icmpv4packet.cpp b/src/icmp/icmpv4packet.cpp index b131be9..20433dc 100644 --- a/src/icmp/icmpv4packet.cpp +++ b/src/icmp/icmpv4packet.cpp @@ -213,19 +213,63 @@ void Icmpv4Packet::print_destination_unreachable() const } /** - * @brief Read the ICMP packet from the @c istream. + * @brief Read (part of) the ICMP packet from the input stream @a is * * @param is The input stream. * - * @return @c true if the read was successful, or @c false if an error occurred. + * @return result of the read, currently one of {ok, fail, not enough data} */ -bool Icmpv4Packet::read( istream &is ) +IcmpPacket::ReadReturnCode Icmpv4Packet::read( istream &is ) { - is.clear(); + if ( !is.good() ) + return IcmpPacket::ReadReturnCode_BAD_STREAM; - is >> *this; + // try to read ip header + is >> IpHeader; - return !is.fail(); + if ( is.eof() ) + return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA; + else if ( !is.good() ) + return IcmpPacket::ReadReturnCode_BAD_STREAM; + + // try to read the icmp header + is >> IcmpPayloadHeader; + + if ( is.eof() ) + return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA; + else if ( !is.good() ) + return IcmpPacket::ReadReturnCode_BAD_STREAM; + + // calculate the size of the ICMP data + streamsize data_length = static_cast( IpHeader.get_total_length() ) + - static_cast( IpHeader.get_header_length() ) + - static_cast( IcmpPayloadHeader.get_header_length() ); + + // try to read that amount of data + if ( data_length < 0 ) + { + GlobalLogger.error() << "Error: invalid size for optional ICMP data: " + << data_length << endl; + is.setstate( ios::failbit ); + return IcmpPacket::ReadReturnCode_INVALID_SIZE; + } + else if ( data_length > 0 ) + { + size_t options_size = static_cast( data_length ); + scoped_array scoped_data( new uint8_t[options_size+1] ); // need a 0 after data + memset(scoped_data.get(), 0, (options_size+1)*sizeof(uint8_t)); + char *char_data = reinterpret_cast( scoped_data.get() ); + + (void) is.read( char_data, data_length ); // leave 0 at the end + IcmpPayloadData = char_data; // implicit cast + } + + if ( is.eof() ) + return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA; + else if ( !is.good() ) + return IcmpPacket::ReadReturnCode_BAD_STREAM; + else + return IcmpPacket::ReadReturnCode_OK; } /** @@ -235,57 +279,12 @@ bool Icmpv4Packet::read( istream &is ) * * @return @c true if the write was successful, or @c false if an error occurred. */ -bool Icmpv4Packet::write( ostream &os ) const +bool Icmpv4Packet::write( std::ostream &os ) const { os.clear(); - os << *this; + os << IcmpPayloadHeader << IcmpPayloadData; return !os.fail(); } -istream& operator>>( - istream &is, - Icmpv4Packet &packet -) -{ - if (is.good()) - is >> packet.IpHeader; - if (is.good()) - is >> packet.IcmpPayloadHeader; - if (is.good()) - { - streamsize data_length = static_cast( packet.IpHeader.get_total_length() ) - - static_cast( packet.IpHeader.get_header_length() ) - - static_cast( packet.IcmpPayloadHeader.get_header_length() ); - - if ( data_length < 0 ) - { - GlobalLogger.error() << "Error: invalid size for optional ICMP data: " - << data_length << endl; - is.setstate( ios::failbit ); - } - else if ( data_length > 0 ) - { - size_t options_size = static_cast( data_length ); - scoped_array scoped_data( new uint8_t[options_size+1] ); // need a 0 after data - memset(scoped_data.get(), 0, (options_size+1)*sizeof(uint8_t)); - char *char_data = reinterpret_cast( scoped_data.get() ); - - (void) is.read( char_data, data_length ); - packet.IcmpPayloadData = char_data; - } - } - - return is; -} - -ostream& operator<<( - ostream& os, - const Icmpv4Packet& packet -) -{ - os << packet.IcmpPayloadHeader << packet.IcmpPayloadData; - - return os; -} diff --git a/src/icmp/icmpv4packet.h b/src/icmp/icmpv4packet.h index ac1c7f0..1c0d075 100644 --- a/src/icmp/icmpv4packet.h +++ b/src/icmp/icmpv4packet.h @@ -97,17 +97,9 @@ public: ) const; virtual void print_destination_unreachable() const; - virtual bool read( std::istream &is ); - virtual bool write( std::ostream &os ) const; + virtual ReadReturnCode read( std::istream &is ); + bool write( std::ostream &os ) const; - friend std::istream& operator>>( - std::istream &is, - Icmpv4Packet &packet - ); - friend std::ostream& operator<<( - std::ostream &os, - const Icmpv4Packet &packet - ); private: bool match( diff --git a/src/icmp/icmpv6packet.cpp b/src/icmp/icmpv6packet.cpp index 5d22e90..5545add 100644 --- a/src/icmp/icmpv6packet.cpp +++ b/src/icmp/icmpv6packet.cpp @@ -9,6 +9,7 @@ #include #include +#include #include "boost_assert_handler.h" @@ -17,6 +18,7 @@ using boost::asio::ip::address; using boost::date_time::time_resolution_traits_adapted64_impl; using boost::posix_time::ptime; using boost::posix_time::microsec_clock; +using boost::scoped_array; using I2n::Logger::GlobalLogger; //----------------------------------------------------------------------------- @@ -224,19 +226,65 @@ void Icmpv6Packet::print_destination_unreachable() const } /** - * @brief Read the ICMP packet from the @c istream. + * @brief Read (part of) the ICMP packet from the input stream @a is * * @param is The input stream. * - * @return @c true if the read was successful, or @c false if an error occurred. + * @return result of the read, currently one of {ok, fail, not enough data} */ -bool Icmpv6Packet::read( istream &is ) +IcmpPacket::ReadReturnCode Icmpv6Packet::read( istream &is ) { - is.clear(); + if ( !is.good() ) + return IcmpPacket::ReadReturnCode_BAD_STREAM; - is >> *this; +#ifdef IPV6_DATA_PRESENT_IN_ISTREAM + //TODO WHY IPv6 does not come like IPv4???? + // try to read ip header + is >> IpHeader; + + if ( is.eof() ) + return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA; + else if ( !is.good() ) + return IcmpPacket::ReadReturnCode_BAD_STREAM; +#endif - return !is.fail(); + // try to read the icmp header + is >> IcmpPayloadHeader; + + if ( is.eof() ) + return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA; + else if ( !is.good() ) + return IcmpPacket::ReadReturnCode_BAD_STREAM; + + // calculate the size of the ICMP data + streamsize data_length = static_cast( IpHeader.get_payload_length() ) + - static_cast( IcmpPayloadHeader.get_header_length() ); + + // try to read that amount of data + if ( data_length < 0 ) + { + GlobalLogger.error() << "Error: invalid size for optional ICMP data: " + << data_length << endl; + is.setstate( ios::failbit ); + return IcmpPacket::ReadReturnCode_INVALID_SIZE; + } + else if ( data_length > 0 ) + { + size_t options_size = static_cast( data_length ); + scoped_array scoped_data( new uint8_t[options_size+1] ); // need a 0 after data + memset(scoped_data.get(), 0, (options_size+1)*sizeof(uint8_t)); + char *char_data = reinterpret_cast( scoped_data.get() ); + + (void) is.read( char_data, data_length ); // leave 0 at the end + IcmpPayloadData = char_data; // implicit cast + } + + if ( is.eof() ) + return IcmpPacket::ReadReturnCode_NOT_ENOUGH_DATA; + else if ( !is.good() ) + return IcmpPacket::ReadReturnCode_BAD_STREAM; + else + return IcmpPacket::ReadReturnCode_OK; } /** @@ -246,44 +294,12 @@ bool Icmpv6Packet::read( istream &is ) * * @return @c true if the write was successful, or @c false if an error occurred. */ -bool Icmpv6Packet::write( ostream &os ) const +bool Icmpv6Packet::write( std::ostream &os ) const { os.clear(); - os << *this; + os << IcmpPayloadHeader << IcmpPayloadData; return !os.fail(); } -istream& operator>>( - istream &is, - Icmpv6Packet &packet -) -{ -#ifdef IPV6_DATA_PRESENT_IN_ISTREAM - //TODO WHY IPv6 does not come like IPv4???? - if (is.good()) - is >> packet.IpHeader; - if (is.good()) - is >> packet.IcmpPayloadHeader; - if (is.good()) - is >> packet.IcmpPayloadData; -#else - if (is.good()) - is >> packet.IcmpPayloadHeader; - if (is.good()) - is >> packet.IcmpPayloadData; -#endif - - return is; -} - -ostream& operator<<( - ostream& os, - const Icmpv6Packet& packet -) -{ - os << packet.IcmpPayloadHeader << packet.IcmpPayloadData; - - return os; -} diff --git a/src/icmp/icmpv6packet.h b/src/icmp/icmpv6packet.h index 5d0bd69..0947398 100644 --- a/src/icmp/icmpv6packet.h +++ b/src/icmp/icmpv6packet.h @@ -111,17 +111,9 @@ public: ) const; virtual void print_destination_unreachable() const; - virtual bool read( std::istream &is ); - virtual bool write( std::ostream &os ) const; + virtual ReadReturnCode read( std::istream &is ); + bool write( std::ostream &os ) const; - friend std::istream& operator>>( - std::istream &is, - Icmpv6Packet &packet - ); - friend std::ostream& operator<<( - std::ostream &os, - const Icmpv6Packet &packet - ); private: bool match( -- 1.7.1