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

Examples 6, 7, 22 and 24 are wrong #310

Closed
ibc opened this issue Dec 26, 2015 · 5 comments
Closed

Examples 6, 7, 22 and 24 are wrong #310

ibc opened this issue Dec 26, 2015 · 5 comments

Comments

@ibc
Copy link
Contributor

@ibc ibc commented Dec 26, 2015

  RTCCertificate.generateCertificate(keygenAlgorithm).then(function(certificate){
    var cert = certificate;
    // Obtain the fingerprint of the created certificate
    dtlsParameters.fingerprints[0] = cert.fingerprint;
  }, function(){
    trace('Certificate could not be created');
  });
  // Prepare to handle remote ICE candidates
  mySignaller.onRemoteCandidate = function(remote) {
    // Figure out which IceTranport a remote candidate relates to by matching the userNameFragment/password
    var j = 0;
    for (j = 0; j < iceTransport.length; j++) {
      var transport = iceTransports[j];
      if (transport.getRemoteParameters().userNameFragment === remote.parameters.userNameFragment)
        transport.addRemoteCandidate(remote.candidate);
      }
    }  };
  // ... create RtpSender/RtpReceiver objects as illustrated in Section 6.5 Example 9.

  mySignaller.mySendInitiate({
    "ice": iceGatherer.getLocalParameters(),
    "dtls": dtlsParameters,
    // ... marshall RtpSender/RtpReceiver capabilities as illustrated in Section 6.5 Example 9.
  }, function(remote) {
    // Create the ICE and DTLS transports
    var iceTransport = new RTCIceTransport(iceGatherer);
    iceTransport.start(iceGatherer, remote.ice, RTCIceRole.controlling);
    iceTransports.push(iceTransport);
    // Construct a RTCDtlsTransport object with the same certificate and fingerprint as in the Offer
    // so that the remote peer can verify it.
    var dtlsTransport = new RTCDtlsTransport(iceTransport, cert);     // <---- There is no `cert` var
    dtlsTransport.start(remote.dtls);
    dtlsTransports.push(dtlsTransport);

    // ... configure RtpSender/RtpReceiver objects as illustrated in Section 6.5 Example 9.
  });

In var dtlsTransport = new RTCDtlsTransport(iceTransport, cert); such a cert variable does not exist in that scope. It was defined in the resolve function of the RTCCertificate.generateCertificate() returned Promise.

For this to be valid, var cert should be declared above the RTCCertificate.generateCertificate() call.

@ibc
Copy link
Contributor Author

@ibc ibc commented Dec 26, 2015

Some issue in Example 7:

  // Create the DTLS certificate
  var keygenAlgorithm = { name: "ECDSA", namedCurve: "P-256" };
  RTCCertificate.generateCertificate(keygenAlgorithm).then(function(certificate){
    var cert = certificate;
  }, function(){
    trace('Certificate could not be created');
  });

  // Create ICE and DTLS transports
  var ice = new RTCIceTransport(iceGatherer);
  var dtls = new RTCDtlsTransport(ice, cert);   // `cert` does not exist in this scope.
@ibc
Copy link
Contributor Author

@ibc ibc commented Dec 26, 2015

If I do not miss anything I will send a PR next week,

@aboba aboba added the 1.1 label Dec 26, 2015
@aboba aboba changed the title Example 6 is wrong Examples 6 and 7 are wrong Dec 26, 2015
aboba added a commit that referenced this issue Dec 26, 2015
@aboba
Copy link
Contributor

@aboba aboba commented Dec 26, 2015

PR for this issue: #312

@ibc
Copy link
Contributor Author

@ibc ibc commented Dec 27, 2015

Great, didn't see the PR.

aboba added a commit that referenced this issue Jan 2, 2016
@aboba
Copy link
Contributor

@aboba aboba commented Jan 4, 2016

Found similar problems in Examples 22 and 24. PR submitted to fix those as well as some respec errors: #319

@aboba aboba changed the title Examples 6 and 7 are wrong Examples 6, 7, 22 and 24 are wrong Jan 4, 2016
@aboba aboba closed this Jan 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants