Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implemented and exposed rejection reason #720

Merged
merged 11 commits into from
Aug 14, 2019

Conversation

ethouris
Copy link
Collaborator

@ethouris ethouris commented Jun 4, 2019

The rejection reason has been implemented. As a demonstration, also reported by srt-test-live.

Internal changes: the codes for m_iReqType in the handshake contain error values that contain reject reason, which occupy all values above 1000 (as previously erroneous values). Backward compatibility assignments to codes 1002 and 1004 as SRT_REJ_PEER and SRT_REJ_ROGUE respectively, however when getting the rejection reason from the peer in older version than this PR will get always SRT_REJ_PEER code, only newer version will report correct detailed code.

API changes: new API entries:

srt_getrejectreason(SRTSOCKET): call this function immediately after the failure has reported the error code as SRT_ECONNREJ. This gets the reject reason as a numeric value, however potentially with the SRT_REJ_PEERREP bit set, which means that this code has been reported by peer, not by agent.

srt_rejectreason_str(enum SRT_REJECT_REASON): will return the message code, disregarding the SRT_REJ_PEERREP bit - this one must be checked separately (this is a result from the simplicity of the implementation, where the code is mapped to a message in an array).

Fixes #684

@maxsharabayko maxsharabayko added this to the v.1.3.4 milestone Jun 4, 2019
@ethouris ethouris modified the milestones: v.1.3.4, v.1.3.3 Jun 4, 2019
@ethouris ethouris added [core] Area: Changes in SRT library core Status: Review Needed labels Jun 4, 2019
@ethouris ethouris modified the milestones: v.1.3.3, v.1.3.4 Jun 4, 2019
@ethouris
Copy link
Collaborator Author

What I'm not sure about, after time, is that whether this "peer reported" flag is really required. This complicates the API usage for the user, while for the user it usually doesn't matter if the rejection was initiated by the listener or the caller, moreover, usually it will be the listener that rejected the connection, while this can only be reported on the caller. I'm thinking of removing this bit completely as unnecessary.

Copy link
Collaborator

@maxsharabayko maxsharabayko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider renaming the SRT_REJ_STRICTENC, given that strict encryption is also to be renamed (#755).
Maybe SRT_REJ_BADENCRYPT?

@maxsharabayko
Copy link
Collaborator

Usage example:

SRTSOCKET sock = srt_create_socket();
struct sockaddr_in sa;    // Fill in the structure

const int connect_res = srt_connect(sock, (struct sockaddr*)&sa, sizeof sa); 
if (connect_res == SRT_ERROR)
{
	const int error_code = srt_getlasterror();
	if (error_code == SRT_ECONNREJ)
	{
		const SRT_REJECT_REASON reason = srt_getrejectreason(m_sock);
		const char* reason_str = srt_rejectreason_str(reason); 
		std::cerr << "Connection rejected: " << reason_str << "\n";
	}
}

@ethouris
Copy link
Collaborator Author

ethouris commented Aug 5, 2019

Consider renaming the SRT_REJ_STRICTENC, given that strict encryption is also to be renamed (#755).
Maybe SRT_REJ_BADENCRYPT?

I think SRT_REJ_UNSECURE is good enough.

@maxsharabayko maxsharabayko added Type: Enhancement Indicates new feature requests and removed Status: Review Needed labels Aug 13, 2019
@rndi rndi merged commit 887bb6b into Haivision:master Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Type: Enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FeatureRequest] Reason for rejected connection
3 participants