Skip to main content
Code Review

Return to Answer

replaced http://tools.ietf.org/html/rfc with https://www.rfc-editor.org/rfc/rfc
Source Link

This is doomed to failure:

struct StringArg
{
 int length;
 char* data;
 StringArg(const char* inData)
 {
 length = strlen(inData);
 data = new char[length+1];
 strcpy(data, inData);
 }
};

This can only lead to disaster. It is unlikely it is more efficient than std::string and is much more likely to lead to memory leaks.

Use std::string it will work is correct and has been optimized very heavily you can not beat it using your own hand written one. This is a premature optimization.

Same Again:

struct StringReturn
{
 int size;
 char* data;
 StringReturn(int inSize)
 {
 size = inSize;
 data = new char[size+1];
 }
};

Don't pass pointers around:

StringArg* stringArg(char* line)
{
 return new StringArg(line);
}

Use std::unique_ptr to encapsulate the pointer object so that it does correct memory management. If you are doing things correctly the resulting code optimizes to a no-op and if you don't do things correctly it saves your ass. In fact I would just throw away your string classes and use std::string. The copy construction from a return is nearly always elided by NRVO and thus is much more efficient than you seem to think.

Same Again

StringReturn* stringReturnInfo(int size)
{
 return new StringReturn(size);
}

Manually copying a C-string is very painful. Use std::string

 int lengths = str->length;
 char* urlData = new char[lengths+1];
 char* fmt = new char[4];
 memcpy(urlData, str->data, str->length);
 urlData[lengths] = 0;
 // Or you can do:
 std::string urlData = line;

Also your code is not exception safe.
Please use RIAA to make sure that there are no leaks (or use std::string that does this already).

The definition of a URL can be found here: http://tools.ietf.org/html/rfc3986 https://www.rfc-editor.org/rfc/rfc3986
Pretending the '+' is a space is only valid in the path section of a URI and not valid in the query or fragment parts of the URI thus scanning the whole string for '+' is not correct.

I also need to completely decode the string. I am aware of double encoded attacks

The specification does not allow for double (or nested) encoding. If it decodes to another '%' then that is the value. You don't try and re-interpret that value.

This is doomed to failure:

struct StringArg
{
 int length;
 char* data;
 StringArg(const char* inData)
 {
 length = strlen(inData);
 data = new char[length+1];
 strcpy(data, inData);
 }
};

This can only lead to disaster. It is unlikely it is more efficient than std::string and is much more likely to lead to memory leaks.

Use std::string it will work is correct and has been optimized very heavily you can not beat it using your own hand written one. This is a premature optimization.

Same Again:

struct StringReturn
{
 int size;
 char* data;
 StringReturn(int inSize)
 {
 size = inSize;
 data = new char[size+1];
 }
};

Don't pass pointers around:

StringArg* stringArg(char* line)
{
 return new StringArg(line);
}

Use std::unique_ptr to encapsulate the pointer object so that it does correct memory management. If you are doing things correctly the resulting code optimizes to a no-op and if you don't do things correctly it saves your ass. In fact I would just throw away your string classes and use std::string. The copy construction from a return is nearly always elided by NRVO and thus is much more efficient than you seem to think.

Same Again

StringReturn* stringReturnInfo(int size)
{
 return new StringReturn(size);
}

Manually copying a C-string is very painful. Use std::string

 int lengths = str->length;
 char* urlData = new char[lengths+1];
 char* fmt = new char[4];
 memcpy(urlData, str->data, str->length);
 urlData[lengths] = 0;
 // Or you can do:
 std::string urlData = line;

Also your code is not exception safe.
Please use RIAA to make sure that there are no leaks (or use std::string that does this already).

The definition of a URL can be found here: http://tools.ietf.org/html/rfc3986
Pretending the '+' is a space is only valid in the path section of a URI and not valid in the query or fragment parts of the URI thus scanning the whole string for '+' is not correct.

I also need to completely decode the string. I am aware of double encoded attacks

The specification does not allow for double (or nested) encoding. If it decodes to another '%' then that is the value. You don't try and re-interpret that value.

This is doomed to failure:

struct StringArg
{
 int length;
 char* data;
 StringArg(const char* inData)
 {
 length = strlen(inData);
 data = new char[length+1];
 strcpy(data, inData);
 }
};

This can only lead to disaster. It is unlikely it is more efficient than std::string and is much more likely to lead to memory leaks.

Use std::string it will work is correct and has been optimized very heavily you can not beat it using your own hand written one. This is a premature optimization.

Same Again:

struct StringReturn
{
 int size;
 char* data;
 StringReturn(int inSize)
 {
 size = inSize;
 data = new char[size+1];
 }
};

Don't pass pointers around:

StringArg* stringArg(char* line)
{
 return new StringArg(line);
}

Use std::unique_ptr to encapsulate the pointer object so that it does correct memory management. If you are doing things correctly the resulting code optimizes to a no-op and if you don't do things correctly it saves your ass. In fact I would just throw away your string classes and use std::string. The copy construction from a return is nearly always elided by NRVO and thus is much more efficient than you seem to think.

Same Again

StringReturn* stringReturnInfo(int size)
{
 return new StringReturn(size);
}

Manually copying a C-string is very painful. Use std::string

 int lengths = str->length;
 char* urlData = new char[lengths+1];
 char* fmt = new char[4];
 memcpy(urlData, str->data, str->length);
 urlData[lengths] = 0;
 // Or you can do:
 std::string urlData = line;

Also your code is not exception safe.
Please use RIAA to make sure that there are no leaks (or use std::string that does this already).

The definition of a URL can be found here: https://www.rfc-editor.org/rfc/rfc3986
Pretending the '+' is a space is only valid in the path section of a URI and not valid in the query or fragment parts of the URI thus scanning the whole string for '+' is not correct.

I also need to completely decode the string. I am aware of double encoded attacks

The specification does not allow for double (or nested) encoding. If it decodes to another '%' then that is the value. You don't try and re-interpret that value.

Source Link
Loki Astari
  • 97.7k
  • 5
  • 126
  • 341

This is doomed to failure:

struct StringArg
{
 int length;
 char* data;
 StringArg(const char* inData)
 {
 length = strlen(inData);
 data = new char[length+1];
 strcpy(data, inData);
 }
};

This can only lead to disaster. It is unlikely it is more efficient than std::string and is much more likely to lead to memory leaks.

Use std::string it will work is correct and has been optimized very heavily you can not beat it using your own hand written one. This is a premature optimization.

Same Again:

struct StringReturn
{
 int size;
 char* data;
 StringReturn(int inSize)
 {
 size = inSize;
 data = new char[size+1];
 }
};

Don't pass pointers around:

StringArg* stringArg(char* line)
{
 return new StringArg(line);
}

Use std::unique_ptr to encapsulate the pointer object so that it does correct memory management. If you are doing things correctly the resulting code optimizes to a no-op and if you don't do things correctly it saves your ass. In fact I would just throw away your string classes and use std::string. The copy construction from a return is nearly always elided by NRVO and thus is much more efficient than you seem to think.

Same Again

StringReturn* stringReturnInfo(int size)
{
 return new StringReturn(size);
}

Manually copying a C-string is very painful. Use std::string

 int lengths = str->length;
 char* urlData = new char[lengths+1];
 char* fmt = new char[4];
 memcpy(urlData, str->data, str->length);
 urlData[lengths] = 0;
 // Or you can do:
 std::string urlData = line;

Also your code is not exception safe.
Please use RIAA to make sure that there are no leaks (or use std::string that does this already).

The definition of a URL can be found here: http://tools.ietf.org/html/rfc3986
Pretending the '+' is a space is only valid in the path section of a URI and not valid in the query or fragment parts of the URI thus scanning the whole string for '+' is not correct.

I also need to completely decode the string. I am aware of double encoded attacks

The specification does not allow for double (or nested) encoding. If it decodes to another '%' then that is the value. You don't try and re-interpret that value.

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /