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 <[email protected]> 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 <[email protected]>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 <[email protected]> wrote:
>>
>>> --- On *Mon, 4/12/10, Phillip Hellewell <[email protected]>* 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;
}
}}}
