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;
}

}}}

Reply via email to