Fix numerous bugs in auxilliary data handling:

Correct undefined behavior due to trying to iterate between to separate temporary objects. 
Clean up extra quote characters inserted by FITS. 
Ignore COMMENT entries added by cfitsio to prevent them from multiplying endlessly. 
Introduce a const-correct character pointer type in case of unusual allocators. 
Stop the user from storing keys which are not allowed in FITS files.
Stop the user from trying to store data which will not fit in a FITS keyword record. 
Add an overload of `splinetable::read_key` which retreives strings including whitespace.

include/photospline/detail/aux.h

@@ -6,8 +6,8 @@
 namespace photospline{
 
 template<typename Alloc>
-const char* splinetable<Alloc>::get_aux_value(const char* key) const{
-	char* value=NULL;
+typename splinetable<Alloc>::const_char_ptr splinetable<Alloc>::get_aux_value(const char* key) const{
+	const_char_ptr value=nullptr;
 	for (uint32_t i=0; i < naux; i++) {
 		// NB: aux[i][0] may be a smart pointer
 		if (strcmp(key, &*aux[i][0]) == 0) {
@@ -61,36 +61,81 @@ bool splinetable<Alloc>::remove_key(const char* key){
 template<typename Alloc>
 template<typename T>
 bool splinetable<Alloc>::read_key(const char* key, T& result) const{
-	const char *value = get_aux_value(key);
+	const_char_ptr value = get_aux_value(key);
 	if(!value)
 		return (false);
-	std::istringstream ss(value);
+	std::istringstream ss(&*value);
 	ss >> result;
 	return (!ss.fail());
 }
 
+template<typename Alloc>
+bool splinetable<Alloc>::read_key(const char* key, std::string& result) const{
+	const_char_ptr value = get_aux_value(key);
+	if(!value)
+		return (false);
+	result=&*value;
+	return (true);
+}
+
 template<typename Alloc>
 template<typename T>
 bool splinetable<Alloc>::write_key(const char* key, const T& value){
 	//check if the key is allowed
 	if (reservedFitsKeyword(key))
 		throw std::runtime_error("Cannot set key with reserved name "+std::string(key));
+	size_t keylen = strlen(key) + 1;
+	size_t maxdatalen=68; //valid for short keys
+	if(keylen<=9){ //up to 8 bytes of data
+		for(size_t i=0; i<keylen-1; i++){
+			if(!(std::isupper(key[i]) || std::isdigit(key[i])) || key[i]=='-' || key[i]=='_')
+				throw std::runtime_error("Standard (short) FITS header keywords are forbidden "
+										 "to contain characters other than uppercase letters, "
+										 "digits, dashes, and underscores (key was '"+
+										 std::string(key)+"')");
+		}
+	}
+	else{
+		//it is unclear what the contraints on the format of long keyword names 
+		//are, since the 'HIERARCH Keyword Convention' document refers to "the 
+		//rules for free-format keywords, as defined in the FITS Standard 
+		//document", when no such rules appear to exist. If this was intended to 
+		//refer to section 4.1.2.1 then cfitsio's behavior of allowing long 
+		//keywords (not split by spaces or periods) at all is non-conforming anyway. 
+		for(size_t i=0; i<keylen-1; i++){
+			if(key[i]=='=')
+				throw std::runtime_error("Standard (short) FITS header keywords must not "
+										 "contain '=' characters (key was '"+
+										 std::string(key)+"')");
+		}
+		maxdatalen=80-(13+keylen-1); //14 characters for "HIERARCH ", "= '", and "'"
+	}
+	std::ostringstream ss;
+	ss << value;
+	if(ss.fail())
+		return(false);
+	std::string valuedata=ss.str();
+	size_t valuelen = valuedata.size() + 1;
+	//For normal (short) keys, we get up to 68 bytes of storage, but for longer keywords
+	//the 'HIERARCH Keyword Convention' kicks in and limits us further
+	if(valuelen-1>maxdatalen){
+		throw std::runtime_error("Value is too long to be stored as a FITS keyword ('"
+								 +valuedata+"' has length "+std::to_string(valuelen-1)
+								 +", but a maximum of "+std::to_string(maxdatalen)+
+								 " characters will fit with this key since continued "
+								 "string keywords are not currently implemented.)");
+	}
 	//check if the key already exists and we should update it
 	size_t i;
 	for (i=0; i < naux; i++) {
 		if (strcmp(key, &*aux[i][0]) == 0)
 			break;
 	}
-	std::ostringstream ss;
-	ss << value;
-	if(ss.fail())
-		return(false);
-	size_t valuelen = ss.str().size() + 1;
 	if (i!=naux) { //the key was found, so update it
 		//try to allocate the correct amount of space for the new value
 		try{
 			char_ptr new_value=allocate<char>(valuelen);
-			std::copy(ss.str().begin(),ss.str().end(),new_value);
+			std::copy(valuedata.begin(),valuedata.end(),new_value);
 			*(new_value+valuelen-1)=0;
 			deallocate(aux[i][1],strlen(&aux[i][1][0])+1);
 			aux[i][1]=new_value;
@@ -101,7 +146,6 @@ bool splinetable<Alloc>::write_key(const char* key, const T& value){
 	}
 	//otherwise, allocate more space and append
 	else {
-		size_t keylen = strlen(key) + 1;
 		//see if we can get space for the new data before we touch any existing things
 		char_ptr new_key=nullptr, new_value=nullptr;
 		char_ptr_ptr new_entry=nullptr;
@@ -125,7 +169,7 @@ bool splinetable<Alloc>::write_key(const char* key, const T& value){
 		new_aux[naux][0] = new_key;
 		new_aux[naux][1] = new_value;
 		std::copy(key,key+keylen,new_aux[naux][0]);
-		std::copy(ss.str().begin(),ss.str().end(),new_value);
+		std::copy(valuedata.begin(),valuedata.end(),new_value);
 		*(new_value+valuelen-1)=0;
 		deallocate(aux,naux);
 		aux = new_aux;

include/photospline/detail/fitsio.h

@@ -219,7 +219,24 @@ bool splinetable<Alloc>::read_fits_core(fitsfile* fits, const std::string& fileP
 				aux[i][0] = allocate<char>(keylen);
 				aux[i][1] = allocate<char>(valuelen);
 				std::copy(key,key+keylen,aux[i][0]);
-				std::copy(value,value+valuelen,aux[i][1]);
+				//remove stupid quotes mandated by FITS, but not removed by cfitsio on reading
+				//Note that we do not attempt to remove whitespace, because we cannot 
+				//distinguish whitespace included by the user and whitespace pointlessly
+				//added by FITS.
+				if(valuelen>1 && value[0]=='\''){
+					if(valuelen>2 && value[valuelen-2]=='\''){ //remove a trailing quote also
+						std::copy(value+1,value+valuelen-2,aux[i][1]);
+						aux[i][1][valuelen-3]='\0';
+					}
+					else{ //just remove an opening quote
+						std::copy(value+1,value+valuelen-1,aux[i][1]);
+						aux[i][1][valuelen-2]='\0';
+					}
+				}
+				else{
+					std::copy(value,value+valuelen,aux[i][1]);
+					aux[i][1][valuelen-1]='\0';
+				}
 				i++;
 			}
 		} else {

include/photospline/splinetable.h

@@ -131,6 +131,7 @@ class splinetable{
 	typedef typename allocator_traits::template rebind_traits<double>::pointer double_ptr;
 	typedef typename allocator_traits::template rebind_traits<double_ptr>::pointer double_ptr_ptr;
 	typedef typename allocator_traits::template rebind_traits<char>::pointer char_ptr;
+	typedef typename allocator_traits::template rebind_traits<const char>::pointer const_char_ptr;
 	typedef typename allocator_traits::template rebind_traits<char_ptr>::pointer char_ptr_ptr;
 	typedef typename allocator_traits::template rebind_traits<char_ptr_ptr>::pointer char_ptr_ptr_ptr;
 
@@ -276,11 +277,11 @@ class splinetable{
 	///Get the count of 'auxiliary' keys
 	size_t get_naux_values() const{ return(naux); }
 	///Directly get a particular auxiliary key
-	const char* get_aux_key(size_t i) const{ return(aux[i][0]); }
+	const_char_ptr get_aux_key(size_t i) const{ return(aux[i][0]); }
 	///Directly get a particular auxiliary value
 	///\param key the key whose value should be fetched
 	///\return the value if key exists, otherwise NULL
-	const char* get_aux_value(const char* key) const;
+	const_char_ptr get_aux_value(const char* key) const;
 	///Delete an auxiliary key and associated value
 	///\returns true if the key existed and was removed
 	bool remove_key(const char* key);
@@ -291,6 +292,13 @@ class splinetable{
 	///        sucessfully parsed into result, otherwise false
 	template<typename T>
 	bool read_key(const char* key, T& result) const;
+	///Look up the value associated with an auxiliary key
+	///\note The data extracted may be padded with extra whitespace if it has
+	///      been read from a FITS file. 
+	///\param key the name of the key to look up
+	///\param result location to store the value if the key is found
+	///\return true if the key was found
+	bool read_key(const char* key, std::string& result) const;
 	///Insert or overwrite an auxiliary key,value pair
 	///\param key the name of the key to store
 	///\param value the value to store for the key

src/core/fitsio.cpp

@@ -33,7 +33,8 @@ bool reservedFitsKeyword(const char* key){
 	       strncmp("ORDER", key, 5) == 0 ||
 	       strncmp("NAXIS", key, 5) == 0 ||
 	       strncmp("PERIOD", key, 6) == 0 ||
-	       strncmp("EXTEND", key, 6) == 0);
+	       strncmp("EXTEND", key, 6) == 0 ||
+	       strncmp("COMMENT", key, 7) == 0);
 }
 
 uint32_t countAuxKeywords(fitsfile* fits){

test/test_fitsio.cpp

@@ -48,17 +48,28 @@ void compare_splines(const photospline::splinetable<>& spline1,
 
 TEST(write_fits_spline){
 	photospline::splinetable<> spline("test_data/test_spline_4d.fits");
+	spline.write_key("SHORTKEY",123);
+	spline.write_key("ALongerKey","a string of text");
 
 	spline.write_fits("write_test_spline.fits");
 	photospline::splinetable<> spline2("write_test_spline.fits");
 
 	compare_splines(spline,spline2);
 
+	int extra_key=0;
+	ENSURE(spline2.read_key("SHORTKEY",extra_key));
+	ENSURE_EQUAL(extra_key,123,"Additional keys must survive FITS serialization");
+	std::string extra_key2;
+	ENSURE(spline2.read_key("ALongerKey",extra_key2));
+	ENSURE_EQUAL(extra_key2,"a string of text","Additional keys must survive FITS serialization");
+
 	unlink("write_test_spline.fits");
 }
 
 TEST(fits_mem_spline){
 	photospline::splinetable<> spline("test_data/test_spline_4d.fits");
+	spline.write_key("SHORTKEY",123);
+	spline.write_key("ALongerKey","a string of text");
 
 	auto buffer=spline.write_fits_mem();
 	std::unique_ptr<void,void(*)(void*)> data(buffer.first,&free);
@@ -67,4 +78,11 @@ TEST(fits_mem_spline){
 	spline2.read_fits_mem(data.get(),buffer_size);
 
 	compare_splines(spline,spline2);
+
+	int extra_key=0;
+	ENSURE(spline2.read_key("SHORTKEY",extra_key));
+	ENSURE_EQUAL(extra_key,123,"Additional keys must survive FITS serialization");
+	std::string extra_key2;
+	ENSURE(spline2.read_key("ALongerKey",extra_key2));
+	ENSURE_EQUAL(extra_key2,"a string of text","Additional keys must survive FITS serialization");
 }