Sure enough, it was just a silly bug in strm_gets() that was causing it to mess up on the last line. This fixes it, and now all my unit tests pass!
--- bio_adstream.cpp 2010-04-12 16:27:21.000000000 -0600 +++ bio_adstream_new.cpp 2010-04-12 16:25:24.000000000 -0600 @@ -127,9 +127,9 @@ int ret = strm_read(b, str, len); if( ret <= 0 ) return ret; - if( ret > len ) - ret = len; - str[ret-1] = '\0'; + if( ret >= len ) + ret = len-1; + str[ret] = '\0'; char* newline_pos = std::strchr(str, '\n'); if( newline_pos != NULL ) { Anyway, I still don't like how I implemented strm_gets() so inefficiently, but I'll try to fix it up later. I'm re-attaching the file since the first one I sent had inconsistent line endings. Phillip On Mon, Apr 12, 2010 at 4:14 PM, Phillip Hellewell <ssh...@gmail.com> wrote: > Alright, here's the code. There's a bug in strm_gets() or something > because PEM_read_bio_X509() is still not working.. > > Phillip > > > On Mon, Apr 12, 2010 at 3:43 PM, Phillip Hellewell <ssh...@gmail.com>wrote: > >> Thanks. That's good advice. BTW, I've finished implementing everything >> except gets(). Other implementations I've seen seem to be able to get away >> with not having it, but my unit tests are failing because of it. >> >> Phillip >> >> >> On Mon, Apr 12, 2010 at 2:47 PM, Ray Satiro <raysat...@yahoo.com> wrote: >> >>> --- On *Mon, 4/12/10, Phillip Hellewell <ssh...@gmail.com>* wrote: >>> >>> >>> Do you think it's ok to set my type to just BIO_TYPE_SOURCE_SINK, or do I >>> need to set it to >>> (BIO_TYPE_SOURCE_SINK | >>> some_magic_number_that_hopefully_noone_else_is_using)? >>> >>> Phillip >>> >>> Really it depends what's being called whether you are going to keep it >>> generic or there's some advantage to imitate a BIO_TYPE_FILE or something. >>> Based on what you've described you could imitate a specific type, but the >>> disadvantage to that is that if/when OpenSSL dev team changes that specific >>> type you could have a problem, like say ptr wasn't in use but then they >>> changed the type so it is. Also I really don't see any advantage to making >>> your own magic number, because dev team could just end up using that number >>> at some point and again problem. >>> >>> >>> >> >
// Enable pre-compiled headers when turned on by project. // Has no effect otherwise (includes the empty stdafx.h in ad/). #include <stdafx.h> // bio_adstream.cpp // Copyright (c) 2010 AccessData Corp. #include "bio_adstream.h" #include <cstring> // std::strlen, std::strchr #include <shared/core/ptype.h> // ad::dynamic_ptr_cast #include <shared/io/line_reader.h> namespace ad { namespace shared { namespace crypto { namespace { // unnamed namespace for file-level statics // Forward decls. int strm_create(BIO* b); int strm_destroy(BIO* b); int strm_read(BIO* b, char* buf, int len); int strm_write(BIO* b, const char* buf, int len); int strm_puts(BIO* b, const char* str); int strm_gets(BIO* b, char* str, int len); long strm_ctrl(BIO* b, int cmd, long num, void* ptr); // Callback structure. static BIO_METHOD strm_method = { BIO_TYPE_SOURCE_SINK, "AD stream", strm_write, strm_read, strm_puts, strm_gets, strm_ctrl, strm_create, strm_destroy, NULL, }; BIO_METHOD* BIO_s_adstream() { return &strm_method; } // A simple object to hold onto the stream's shared pointer, // so when this object gets destroyed, the shared pointer's // refcount will decrease, and the stream may be destroyed. struct StreamPtrHolder { StreamPtrHolder(io::file_read_random::ptr strm) : strm(strm) {} io::file_read_random::ptr strm; }; int strm_create(BIO* b) { b->init = 1; b->shutdown = 1; b->num = 0; b->ptr = NULL; b->flags = 0; return 1; } int strm_destroy(BIO* b) { if( b == NULL ) return 0; if( b->init && b->shutdown ) delete reinterpret_cast<StreamPtrHolder*>(b->ptr); b->ptr = NULL; b->init = 0; b->flags = 0; return 1; } int strm_read(BIO* b, char* buf, int len) { if( b == NULL || b->ptr == NULL || buf == NULL || len < 0 ) return -1; io::file_read_random::ptr strm = reinterpret_cast<StreamPtrHolder*>(b->ptr)->strm; if( !strm ) return -1; if( len == 0 ) return 0; return static_cast<int>(strm->read(buf, len)); } int strm_write(BIO* b, const char* buf, int len) { if( b == NULL || b->ptr == NULL || buf == NULL || len < 0 ) return -1; io::file_read_random::ptr strm = reinterpret_cast<StreamPtrHolder*>(b->ptr)->strm; if( !strm ) return -1; io::file_read_write::ptr strm_writable = ad::dynamic_ptr_cast<io::file_read_write>(strm); if( !strm_writable ) return -1; if( len == 0 ) return 0; return static_cast<int>(strm_writable->write(buf, len)); } int strm_puts(BIO* b, const char* str) { return strm_write(b, str, std::strlen(str)); } int strm_gets(BIO* b, char* str, int len) { if( len <= 0 ) return -1; long oldpos = strm_ctrl(b, BIO_C_FILE_TELL, 0, 0); if( oldpos < 0 ) return -1; // FIXME: If their buffer is very large, we could waste time // reading a lot of extra stuff here. Use line_reader? int ret = strm_read(b, str, len); if( ret <= 0 ) return ret; if( ret >= len ) ret = len-1; str[ret] = '\0'; char* newline_pos = std::strchr(str, '\n'); if( newline_pos != NULL ) { *newline_pos = '\0'; ret = (newline_pos - str); } // Put the stream at the next line. Pray it works. strm_ctrl(b, BIO_C_FILE_SEEK, oldpos + ret + 1, 0); // Chop off trailing CR if present, just cuz I'm so nice. if( ret > 0 && str[ret-1] == '\r' ) str[--ret] = '\0'; return ret; } long strm_ctrl(BIO* b, int cmd, long num, void* ptr) { if( b == NULL || b->ptr == NULL ) return -1; io::file_read_random::ptr strm = reinterpret_cast<StreamPtrHolder*>(b->ptr)->strm; if( !strm ) return -1; switch( cmd ) { case BIO_CTRL_EOF: return strm->eof() ? 1 : 0; case BIO_CTRL_RESET: case BIO_C_FILE_SEEK: return static_cast<long>(strm->seek(num)); case BIO_C_FILE_TELL: case BIO_CTRL_INFO: return static_cast<long>(strm->tell()); case BIO_CTRL_PENDING: case BIO_CTRL_WPENDING: return 0; case BIO_CTRL_DUP: case BIO_CTRL_FLUSH: return 1; } return 0; } } // end unnamed namespace for file-level statics BIO* BIO_new_adstream(io::file_read_random::ptr strm) { if( !strm ) return NULL; BIO* ret = BIO_new(BIO_s_adstream()); if( !ret ) return NULL; ret->ptr = new StreamPtrHolder(strm); ret->num = 0; return ret; } }}}