From d5795b3114646d5c6b71338060247c9e78af2b7a Mon Sep 17 00:00:00 2001 From: Lyubomir Marinov Date: Fri, 30 Mar 2012 10:50:27 +0000 Subject: [PATCH] Fixes a ConcurrentModificationException in SRTCPTransformer. --- build.xml | 1 - .../impl/neomedia/MediaStreamImpl.java | 7 +- .../protocol/SourceStreamDelegate.java | 3 +- .../transform/srtp/SRTCPTransformer.java | 19 ++- .../jabber/CallPeerMediaHandlerGTalkImpl.java | 9 +- .../CallPeerMediaHandlerJabberImpl.java | 98 +++++++------ .../impl/protocol/jabber/InfoRetreiver.java | 13 +- .../OperationSetAutoAnswerJabberImpl.java | 35 ++--- .../protocol/jabber/UriHandlerJabberImpl.java | 26 ++-- .../sip/CallPeerMediaHandlerSipImpl.java | 138 ++++++++++-------- .../impl/protocol/sip/CallSipImpl.java | 114 ++++++++------- .../plugin/sipaccregwizz/SecurityPanel.java | 40 ++--- .../neomedia/DefaultTCPStreamConnector.java | 4 +- .../service/neomedia/MediaService.java | 12 +- .../service/neomedia/MediaStreamTarget.java | 5 +- .../service/neomedia/event/SrtpListener.java | 14 +- .../communicator/service/protocol/Call.java | 16 +- .../ChatRoomMemberPresenceChangeEvent.java | 19 +-- .../media/AbstractQualityControlWrapper.java | 4 +- .../protocol/media/MediaAwareCallPeer.java | 11 +- .../protocol/media/TransportManager.java | 60 +++----- .../util/swing/SIPCommDialog.java | 8 +- 22 files changed, 331 insertions(+), 325 deletions(-) diff --git a/build.xml b/build.xml index b702bc9b4..42cb6cdff 100644 --- a/build.xml +++ b/build.xml @@ -872,7 +872,6 @@ - diff --git a/src/net/java/sip/communicator/impl/neomedia/MediaStreamImpl.java b/src/net/java/sip/communicator/impl/neomedia/MediaStreamImpl.java index 462eb1aae..8990bd226 100644 --- a/src/net/java/sip/communicator/impl/neomedia/MediaStreamImpl.java +++ b/src/net/java/sip/communicator/impl/neomedia/MediaStreamImpl.java @@ -274,9 +274,12 @@ public MediaStreamImpl( // TODO Add option to disable ZRTP, e.g. by implementing a NullControl. // If you change the default behavior (initiates a ZrtpControlImpl if // the srtpControl attribute is null), please accordingly modify the - // CallPeerMediaHandler.initStream fucntion. + // CallPeerMediaHandler.initStream function. this.srtpControl - = (srtpControl == null) ? new ZrtpControlImpl() : srtpControl; + = (srtpControl == null) + ? NeomediaActivator.getMediaServiceImpl() + .createZrtpControl() + : srtpControl; if (connector != null) setConnector(connector); diff --git a/src/net/java/sip/communicator/impl/neomedia/protocol/SourceStreamDelegate.java b/src/net/java/sip/communicator/impl/neomedia/protocol/SourceStreamDelegate.java index e995ac84c..add34a47b 100644 --- a/src/net/java/sip/communicator/impl/neomedia/protocol/SourceStreamDelegate.java +++ b/src/net/java/sip/communicator/impl/neomedia/protocol/SourceStreamDelegate.java @@ -12,9 +12,10 @@ * Implements a SourceStream which wraps a specific * SourceStream. * - * @author Lubomir Marinov * @param the very type of the SourceStream wrapped by * SourceStreamDelegate + * + * @author Lyubomir Marinov */ public class SourceStreamDelegate implements SourceStream diff --git a/src/net/java/sip/communicator/impl/neomedia/transform/srtp/SRTCPTransformer.java b/src/net/java/sip/communicator/impl/neomedia/transform/srtp/SRTCPTransformer.java index 5e13146f3..3248619b4 100644 --- a/src/net/java/sip/communicator/impl/neomedia/transform/srtp/SRTCPTransformer.java +++ b/src/net/java/sip/communicator/impl/neomedia/transform/srtp/SRTCPTransformer.java @@ -6,7 +6,7 @@ */ package net.java.sip.communicator.impl.neomedia.transform.srtp; -import java.util.Hashtable; +import java.util.*; import net.java.sip.communicator.impl.neomedia.*; import net.java.sip.communicator.impl.neomedia.transform.*; @@ -103,6 +103,7 @@ public RawPacket reverseTransform(RawPacket pkt) } return pkt; } + /** * Close the transformer and underlying transform engine. * @@ -114,14 +115,18 @@ public void close() forwardEngine.close(); if (forwardEngine != reverseEngine) reverseEngine.close(); - for(Long ssrc : contexts.keySet()) + + Iterator> iter + = contexts.entrySet().iterator(); + + while (iter.hasNext()) { - SRTCPCryptoContext context = contexts.get(ssrc); - if (context != null) - { + Map.Entry entry = iter.next(); + SRTCPCryptoContext context = entry.getValue(); + + iter.remove(); + if (context != null) context.close(); - contexts.remove(ssrc); - } } } } diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java index 830efc35b..f6ea3200c 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerGTalkImpl.java @@ -219,7 +219,7 @@ else if(ext.getNamespace().equals( public void sendCandidates( Iterable candidates) { - getPeer().sendCandidates(candidates); + getPeer().sendCandidates(candidates); } }); } @@ -435,11 +435,8 @@ protected synchronized TransportManagerGTalkImpl getTransportManager() { if (transportManager == null) { - /* Google Talk assumes to use ICE */ - CallPeerGTalkImpl peer = getPeer(); - - // support for Google Talk - transportManager = new TransportManagerGTalkImpl(peer); + // Google Talk assumes the use of ICE. + transportManager = new TransportManagerGTalkImpl(getPeer()); } return transportManager; } diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java index 6e4c83d28..1601c43f0 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/CallPeerMediaHandlerJabberImpl.java @@ -45,7 +45,8 @@ public class CallPeerMediaHandlerJabberImpl private TransportManagerJabberImpl transportManager; /** - * Synchronization object for transportManager instance. + * The Object which is used for synchronization (e.g. wait + * and notify) related to {@link #transportManager}. */ private final Object transportManagerSyncRoot = new Object(); @@ -365,14 +366,17 @@ public void processOffer(List offer) // ZRTP if(getPeer().getCall().isSipZrtpAttribute()) { - MediaTypeSrtpControl key = - new MediaTypeSrtpControl(mediaType, SrtpControlType.ZRTP); - SrtpControl control = getSrtpControls().get(key); + Map srtpControls + = getSrtpControls(); + MediaTypeSrtpControl key + = new MediaTypeSrtpControl(mediaType, SrtpControlType.ZRTP); + SrtpControl control = srtpControls.get(key); + if(control == null) { - control = JabberActivator.getMediaService() - .createZrtpControl(); - getSrtpControls().put(key, control); + control + = JabberActivator.getMediaService().createZrtpControl(); + srtpControls.put(key, control); } String helloHash[] = ((ZrtpControl)control).getHelloHashSep(); @@ -658,15 +662,19 @@ private ContentPacketExtension createContent(MediaDevice dev) //ZRTP if(getPeer().getCall().isSipZrtpAttribute()) { - MediaTypeSrtpControl key = - new MediaTypeSrtpControl(dev.getMediaType(), - SrtpControlType.ZRTP); - SrtpControl control = getSrtpControls().get(key); + Map srtpControls + = getSrtpControls(); + MediaTypeSrtpControl key + = new MediaTypeSrtpControl( + dev.getMediaType(), + SrtpControlType.ZRTP); + SrtpControl control = srtpControls.get(key); + if(control == null) { control = JabberActivator.getMediaService().createZrtpControl(); - getSrtpControls().put(key, control); + srtpControls.put(key, control); } String helloHash[] = ((ZrtpControl)control).getHelloHashSep(); @@ -702,20 +710,12 @@ private ContentPacketExtension createContent(MediaDevice dev) * for reasons like - problems with device interaction, allocating ports, * etc. */ - public ContentPacketExtension createContentForMedia( - MediaType mediaType) + public ContentPacketExtension createContentForMedia(MediaType mediaType) throws OperationFailedException { MediaDevice dev = getDefaultDevice(mediaType); - if (dev != null) - { - ContentPacketExtension content = createContent(dev); - - return content; - } - - return null; + return (dev == null) ? null : createContent(dev); } /** @@ -799,8 +799,9 @@ public List createContentList() if (dev != null) { - MediaDirection direction = dev.getDirection().and( - getDirectionUserPreference(mediaType)); + MediaDirection direction + = dev.getDirection().and( + getDirectionUserPreference(mediaType)); if(isLocallyOnHold()) direction = direction.and(MediaDirection.SENDONLY); @@ -824,15 +825,20 @@ public List createContentList() //ZRTP if(getPeer().getCall().isSipZrtpAttribute()) { - MediaTypeSrtpControl key = - new MediaTypeSrtpControl(mediaType, - SrtpControlType.ZRTP); - SrtpControl control = getSrtpControls().get(key); + Map srtpControls + = getSrtpControls(); + MediaTypeSrtpControl key + = new MediaTypeSrtpControl( + mediaType, + SrtpControlType.ZRTP); + SrtpControl control = srtpControls.get(key); + if(control == null) { - control = JabberActivator.getMediaService() - .createZrtpControl(); - getSrtpControls().put(key, control); + control + = JabberActivator.getMediaService() + .createZrtpControl(); + srtpControls.put(key, control); } String helloHash[] = @@ -882,17 +888,17 @@ public List createContentList() logger); } - TransportInfoSender transportInfoSender = - getTransportManager().getXmlNamespace().equals( - ProtocolProviderServiceJabberImpl.URN_GOOGLE_TRANSPORT_P2P) + TransportInfoSender transportInfoSender + = getTransportManager().getXmlNamespace().equals( + ProtocolProviderServiceJabberImpl.URN_GOOGLE_TRANSPORT_P2P) ? new TransportInfoSender() - { - public void sendTransportInfo( - Iterable contents) - { - getPeer().sendTransportInfo(contents); - } - } + { + public void sendTransportInfo( + Iterable contents) + { + getPeer().sendTransportInfo(contents); + } + } : null; //now add the transport elements @@ -1400,9 +1406,8 @@ private void setTransportManager(String xmlns) CallPeerJabberImpl peer = getPeer(); - if (!peer - .getProtocolProvider() - .getDiscoveryManager().includesFeature(xmlns)) + if (!peer.getProtocolProvider().getDiscoveryManager().includesFeature( + xmlns)) { throw new IllegalArgumentException( "Unsupported Jingle transport " + xmlns); @@ -1561,9 +1566,8 @@ private List harvestCandidates( * We'll be harvesting candidates in order to make an offer so it * doesn't make sense to send them in transport-info. */ - if (!ProtocolProviderServiceJabberImpl - .URN_GOOGLE_TRANSPORT_P2P - .equals(transportManager.getXmlNamespace()) + if (!ProtocolProviderServiceJabberImpl.URN_GOOGLE_TRANSPORT_P2P + .equals(transportManager.getXmlNamespace()) && (transportInfoSender != null)) throw new IllegalArgumentException("transportInfoSender"); diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/InfoRetreiver.java b/src/net/java/sip/communicator/impl/protocol/jabber/InfoRetreiver.java index 1bd17ead5..ad202ea5d 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/InfoRetreiver.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/InfoRetreiver.java @@ -48,15 +48,18 @@ public class InfoRetreiver */ private final long vcardTimeoutReply; - protected InfoRetreiver - (ProtocolProviderServiceJabberImpl jabberProvider, String ownerUin) + protected InfoRetreiver( + ProtocolProviderServiceJabberImpl jabberProvider, + String ownerUin) { this.jabberProvider = jabberProvider; this.ownerUin = ownerUin; - vcardTimeoutReply = JabberActivator.getConfigurationService().getLong( - ProtocolProviderServiceJabberImpl.VCARD_REPLY_TIMEOUT_PROPERTY, - -1); + vcardTimeoutReply + = JabberActivator.getConfigurationService().getLong( + ProtocolProviderServiceJabberImpl + .VCARD_REPLY_TIMEOUT_PROPERTY, + -1); } /** diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/OperationSetAutoAnswerJabberImpl.java b/src/net/java/sip/communicator/impl/protocol/jabber/OperationSetAutoAnswerJabberImpl.java index 88017f087..af07efc5f 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/OperationSetAutoAnswerJabberImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/OperationSetAutoAnswerJabberImpl.java @@ -13,8 +13,8 @@ import java.util.*; /** - * An Operation Set defining option - * to unconditional auto answer incoming calls. + * An Operation Set defining option to unconditionally auto answer incoming + * calls. * * @author Damian Minkov */ @@ -25,8 +25,8 @@ public class OperationSetAutoAnswerJabberImpl /** * Our class logger. */ - private static final Logger logger = - Logger.getLogger(OperationSetAutoAnswerJabberImpl.class); + private static final Logger logger + = Logger.getLogger(OperationSetAutoAnswerJabberImpl.class); /** * Should we unconditionally answer. @@ -36,7 +36,7 @@ public class OperationSetAutoAnswerJabberImpl /** * The parent operation set. */ - private OperationSetBasicTelephonyJabberImpl telephonyJabber; + private final OperationSetBasicTelephonyJabberImpl telephonyJabber; /** * Creates this operation set, loads stored values, populating @@ -45,7 +45,7 @@ public class OperationSetAutoAnswerJabberImpl * @param telephonyJabber the parent opset. */ OperationSetAutoAnswerJabberImpl( - OperationSetBasicTelephonyJabberImpl telephonyJabber) + OperationSetBasicTelephonyJabberImpl telephonyJabber) { this.telephonyJabber = telephonyJabber; @@ -60,8 +60,8 @@ private void load() { AccountID acc = telephonyJabber.getProtocolProvider().getAccountID(); - answerUnconditional = - acc.getAccountPropertyBoolean(AUTO_ANSWER_UNCOND_PROP, false); + answerUnconditional + = acc.getAccountPropertyBoolean(AUTO_ANSWER_UNCOND_PROP, false); } /** @@ -76,9 +76,7 @@ private void save() accProps.put(AUTO_ANSWER_UNCOND_PROP, null); if(answerUnconditional) - { accProps.put(AUTO_ANSWER_UNCOND_PROP, Boolean.TRUE.toString()); - } acc.setAccountProperties(accProps); JabberActivator.getProtocolProviderFactory().storeAccount(acc); @@ -130,22 +128,17 @@ public void clear() * @return true if we have processed and no further processing is * needed, false otherwise. */ - boolean followCallCheck(AbstractCall call) + boolean followCallCheck(AbstractCall call) { if(!answerUnconditional) return false; - // we are here cause we satisfy the conditional, - // or unconditional is true - @SuppressWarnings("unchecked") + // We are here because we satisfy the conditional, or unconditional is + // true. Iterator peers = call.getCallPeers(); while (peers.hasNext()) - { - final CallPeer peer = peers.next(); - - answerPeer(peer); - } + answerPeer(peers.next()); return true; } @@ -160,8 +153,7 @@ private void answerPeer(final CallPeer peer) if (state == CallPeerState.INCOMING_CALL) { - // answer in separate thread, don't block current - // processing + // answer in separate thread, don't block current processing new Thread(new Runnable() { public void run() @@ -190,7 +182,6 @@ public void run() */ public void peerStateChanged(CallPeerChangeEvent evt) { - CallPeerState newState = (CallPeerState) evt.getNewValue(); if (newState == CallPeerState.INCOMING_CALL) diff --git a/src/net/java/sip/communicator/impl/protocol/jabber/UriHandlerJabberImpl.java b/src/net/java/sip/communicator/impl/protocol/jabber/UriHandlerJabberImpl.java index 01597e4d7..b7281d6a5 100644 --- a/src/net/java/sip/communicator/impl/protocol/jabber/UriHandlerJabberImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/jabber/UriHandlerJabberImpl.java @@ -401,25 +401,27 @@ public void handleUri(String uri) * Informs the user that they need to be registered before chatting and * asks them whether they would like us to do it for them. * - * @param uri the uri that the user would like us to chat with after registering. + * @param uri the uri that the user would like us to chat with after + * registering. * @param provider the provider that we may have to reregister. */ - private void promptForRegistration(String uri, - ProtocolProviderService provider) + private void promptForRegistration( + String uri, + ProtocolProviderService provider) { - int answer = - JabberActivator + int answer + = JabberActivator .getUIService() - .getPopupDialog() - .showConfirmPopupDialog( - "You need to be online in order to chat and your " - + "account is currently offline. Do want to connect now?", - "Account is currently offline", PopupDialog.YES_NO_OPTION); + .getPopupDialog() + .showConfirmPopupDialog( + "You need to be online in order to chat and" + + " your account is currently offline. Do" + + " you want to connect now?", + "Account is currently offline", + PopupDialog.YES_NO_OPTION); if (answer == PopupDialog.YES_OPTION) - { new ProtocolRegistrationThread(uri, provider).start(); - } } /** diff --git a/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java b/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java index 3fa3a370d..0aab3541a 100644 --- a/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/sip/CallPeerMediaHandlerSipImpl.java @@ -643,30 +643,33 @@ private Vector createMediaDescriptionsForAnswer( private void updateMediaDescriptionForZrtp( MediaType mediaType, MediaDescription md) { - if(getPeer() - .getProtocolProvider() - .getAccountID() - .getAccountPropertyBoolean( - ProtocolProviderFactory.DEFAULT_ENCRYPTION, true) - && getPeer().getCall().isSipZrtpAttribute()) + MediaAwareCallPeer peer = getPeer(); + + if(peer.getProtocolProvider().getAccountID().getAccountPropertyBoolean( + ProtocolProviderFactory.DEFAULT_ENCRYPTION, + true) + && peer.getCall().isSipZrtpAttribute()) { try { - MediaTypeSrtpControl key = - new MediaTypeSrtpControl(mediaType, SrtpControlType.ZRTP); - SrtpControl scontrol = getSrtpControls().get(key); + Map srtpControls + = getSrtpControls(); + MediaTypeSrtpControl key + = new MediaTypeSrtpControl(mediaType, SrtpControlType.ZRTP); + SrtpControl scontrol = srtpControls.get(key); + if(scontrol == null) { - scontrol = SipActivator.getMediaService() - .createZrtpControl(); - getSrtpControls().put(key, scontrol); + scontrol + = SipActivator.getMediaService().createZrtpControl(); + srtpControls.put(key, scontrol); } - ZrtpControl zcontrol = (ZrtpControl) scontrol; + ZrtpControl zcontrol = (ZrtpControl) scontrol; String helloHash = zcontrol.getHelloHash(); + if(helloHash != null && helloHash.length() > 0) md.setAttribute(SdpUtils.ZRTP_HASH_ATTR, helloHash); - } catch (SdpException ex) { @@ -680,46 +683,44 @@ && getPeer().getCall().isSipZrtpAttribute()) * * @param mediaType the media type. * @param localMd the description of the local peer. - * @param peerMd the description of the remote peer. + * @param remoteMd the description of the remote peer. */ - @SuppressWarnings("unchecked") //jain-sip legacy private boolean updateMediaDescriptionForSDes( - MediaType mediaType, MediaDescription localMd, MediaDescription peerMd) + MediaType mediaType, + MediaDescription localMd, + MediaDescription remoteMd) { + AccountID accountID = getPeer().getProtocolProvider().getAccountID(); + // check if SDES and encryption is enabled at all - if (!getPeer() - .getProtocolProvider() - .getAccountID() - .getAccountPropertyBoolean( - ProtocolProviderFactory.SDES_ENABLED, false) - || - !getPeer() - .getProtocolProvider() - .getAccountID() - .getAccountPropertyBoolean( - ProtocolProviderFactory.DEFAULT_ENCRYPTION, true)) + if (!accountID.getAccountPropertyBoolean( + ProtocolProviderFactory.SDES_ENABLED, + false) + || !accountID.getAccountPropertyBoolean( + ProtocolProviderFactory.DEFAULT_ENCRYPTION, + true)) { return false; } // get or create the control - MediaTypeSrtpControl key = - new MediaTypeSrtpControl(mediaType, SrtpControlType.SDES); - SrtpControl scontrol = getSrtpControls().get(key); + Map srtpControls = getSrtpControls(); + MediaTypeSrtpControl key + = new MediaTypeSrtpControl(mediaType, SrtpControlType.SDES); + SrtpControl scontrol = srtpControls.get(key); + if (scontrol == null) { scontrol = SipActivator.getMediaService().createSDesControl(); - getSrtpControls().put(key, scontrol); + srtpControls.put(key, scontrol); } // set the enabled ciphers suites SDesControl sdcontrol = (SDesControl) scontrol; - String ciphers = - getPeer() - .getProtocolProvider() - .getAccountID() - .getAccountPropertyString( + String ciphers + = accountID.getAccountPropertyString( ProtocolProviderFactory.SDES_CIPHER_SUITES); + if (ciphers == null) { ciphers = @@ -729,29 +730,29 @@ private boolean updateMediaDescriptionForSDes( sdcontrol.setEnabledCiphers(Arrays.asList(ciphers.split(","))); // act as initiator - if (peerMd == null) + if (remoteMd == null) { + @SuppressWarnings("unchecked") Vector atts = localMd.getAttributes(true); + for (String ca : sdcontrol.getInitiatorCryptoAttributes()) - { - Attribute a = SdpUtils.createAttribute("crypto", ca); - atts.add(a); - } + atts.add(SdpUtils.createAttribute("crypto", ca)); + return true; } // act as responder else { - Vector atts = peerMd.getAttributes(true); + @SuppressWarnings("unchecked") + Vector atts = remoteMd.getAttributes(true); List peerAttributes = new LinkedList(); + for (Attribute a : atts) { try { if (a.getName().equals("crypto")) - { peerAttributes.add(a.getValue()); - } } catch (SdpParseException e) { @@ -760,8 +761,9 @@ private boolean updateMediaDescriptionForSDes( } if (peerAttributes.size() > 0) { - String localAttr = - sdcontrol.responderSelectAttribute(peerAttributes); + String localAttr + = sdcontrol.responderSelectAttribute(peerAttributes); + if (localAttr != null) { try @@ -779,9 +781,10 @@ private boolean updateMediaDescriptionForSDes( // none of the offered suites match, destroy the sdes // control sdcontrol.cleanup(); - getSrtpControls().remove(key); - logger.warn("Received unsupported sdes crypto attribute " - + peerAttributes.toString()); + srtpControls.remove(key); + logger.warn( + "Received unsupported sdes crypto attribute " + + peerAttributes); } } else @@ -789,7 +792,7 @@ private boolean updateMediaDescriptionForSDes( // peer doesn't offer any SDES attribute, destroy the sdes // control sdcontrol.cleanup(); - getSrtpControls().remove(key); + srtpControls.remove(key); } return false; } @@ -967,14 +970,18 @@ private synchronized void processAnswer(SessionDescription answer) } // select the crypto key the peer has chosen from our proposal + Map srtpControls + = getSrtpControls(); MediaTypeSrtpControl key = new MediaTypeSrtpControl(mediaType, SrtpControlType.SDES); - SrtpControl scontrol = getSrtpControls().get(key); + SrtpControl scontrol = srtpControls.get(key); + if(scontrol != null) { List peerAttributes = new LinkedList(); @SuppressWarnings("unchecked") Vector attrs = mediaDescription.getAttributes(true); + for (Attribute a : attrs) { try @@ -989,11 +996,11 @@ private synchronized void processAnswer(SessionDescription answer) logger.error("received an unparsable sdp attribute", e); } } - if(!((SDesControl) scontrol) - .initiatorSelectAttribute(peerAttributes)) + if(!((SDesControl) scontrol).initiatorSelectAttribute( + peerAttributes)) { scontrol.cleanup(); - getSrtpControls().remove(key); + srtpControls.remove(key); if(peerAttributes.size() > 0) logger .warn("Received unsupported sdes crypto attribute: " @@ -1002,16 +1009,21 @@ private synchronized void processAnswer(SessionDescription answer) else { //found an SDES answer, remove all other controls - Iterator it = - getSrtpControls().keySet().iterator(); - while (it.hasNext()) + Iterator> iter + = srtpControls.entrySet().iterator(); + + while (iter.hasNext()) { - MediaTypeSrtpControl mtc = it.next(); - if (mtc.mediaType == mediaType - && mtc.srtpControlType != SrtpControlType.SDES) + Map.Entry entry + = iter.next(); + MediaTypeSrtpControl mtsc = entry.getKey(); + + if ((mtsc.mediaType == mediaType) + && (mtsc.srtpControlType + != SrtpControlType.SDES)) { - getSrtpControls().get(mtc).cleanup(); - it.remove(); + entry.getValue().cleanup(); + iter.remove(); } } diff --git a/src/net/java/sip/communicator/impl/protocol/sip/CallSipImpl.java b/src/net/java/sip/communicator/impl/protocol/sip/CallSipImpl.java index 29c919bbf..c02ddd6ce 100644 --- a/src/net/java/sip/communicator/impl/protocol/sip/CallSipImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/sip/CallSipImpl.java @@ -27,8 +27,8 @@ */ public class CallSipImpl extends MediaAwareCall + OperationSetBasicTelephonySipImpl, + ProtocolProviderServiceSipImpl> implements CallPeerListener { /** @@ -97,7 +97,6 @@ public CallPeerSipImpl findCallPeer(Dialog dialog) logger.trace("Looking for peer with dialog: " + dialog + "among " + getCallPeerCount() + " calls"); } - while (callPeers.hasNext()) { CallPeerSipImpl cp = callPeers.next(); @@ -237,29 +236,36 @@ public void reInvite() throws OperationFailedException private CallPeerSipImpl createCallPeerFor( Transaction containingTransaction, SipProvider sourceProvider) { - CallPeerSipImpl callPeer = new CallPeerSipImpl( - containingTransaction.getDialog().getRemoteParty(), - this, containingTransaction, sourceProvider); + CallPeerSipImpl callPeer + = new CallPeerSipImpl( + containingTransaction.getDialog().getRemoteParty(), + this, + containingTransaction, + sourceProvider); + addCallPeer(callPeer); boolean incomingCall = (containingTransaction instanceof ServerTransaction); - callPeer.setState( incomingCall - ? CallPeerState.INCOMING_CALL - : CallPeerState.INITIATING_CALL); + + callPeer.setState( + incomingCall + ? CallPeerState.INCOMING_CALL + : CallPeerState.INITIATING_CALL); // if this was the first peer we added in this call then the call is // new and we also need to notify everyone of its creation. - if(this.getCallPeerCount() == 1) + if(getCallPeerCount() == 1) { - Map mediaDirections = new - HashMap(); + Map mediaDirections + = new HashMap(); mediaDirections.put(MediaType.AUDIO, MediaDirection.INACTIVE); mediaDirections.put(MediaType.VIDEO, MediaDirection.INACTIVE); boolean hasZrtp = false; boolean hasSdes = false; + //this check is not mandatory catch all to skip if a problem exists try { @@ -270,51 +276,51 @@ private CallPeerSipImpl createCallPeerFor( if(inviteReq != null && inviteReq.getRawContent() != null) { String sdpStr = SdpUtils.getContentAsString(inviteReq); - - SessionDescription sesDescr = SdpUtils.parseSdpString(sdpStr); - - List remoteDescriptions = SdpUtils - .extractMediaDescriptions(sesDescr); + SessionDescription sesDescr + = SdpUtils.parseSdpString(sdpStr); + List remoteDescriptions + = SdpUtils.extractMediaDescriptions(sesDescr); for (MediaDescription mediaDescription : remoteDescriptions) { - MediaType mediaType = - SdpUtils.getMediaType(mediaDescription); + MediaType mediaType + = SdpUtils.getMediaType(mediaDescription); - hasZrtp = hasZrtp || mediaDescription.getAttribute( - SdpUtils.ZRTP_HASH_ATTR) != null; + mediaDirections.put( + mediaType, + SdpUtils.getDirection(mediaDescription)); - if(mediaType.equals(MediaType.VIDEO)) + // hasZrtp? + if (!hasZrtp) { - MediaDirection videoDirection = - SdpUtils.getDirection(mediaDescription); - mediaDirections.put(MediaType.VIDEO, - videoDirection); + hasZrtp + = (mediaDescription.getAttribute( + SdpUtils.ZRTP_HASH_ATTR) + != null); } - else if(mediaType.equals(MediaType.AUDIO)) + // hasSdes? + if (!hasSdes) { - MediaDirection audioDirection = - SdpUtils.getDirection(mediaDescription); - mediaDirections.put(MediaType.AUDIO, - audioDirection); - } + @SuppressWarnings("unchecked") + Vector attrs + = mediaDescription.getAttributes(true); - @SuppressWarnings("unchecked") - Vector attrs = - mediaDescription.getAttributes(true); - for (Attribute a : attrs) - { - try + for (Attribute attr : attrs) { - if (a.getName().equals("crypto")) + try { - hasSdes = true; + if ("crypto".equals(attr.getName())) + { + hasSdes = true; + break; + } + } + catch (SdpParseException spe) + { + logger.error( + "Failed to parse SDP attribute", + spe); } - } - catch (SdpParseException e) - { - logger.error( - "received an unparsable sdp attribute", e); } } } @@ -325,25 +331,25 @@ else if(mediaType.equals(MediaType.AUDIO)) logger.warn("Error getting media types", t); } - if(this.getCallGroup() == null) + if(getCallGroup() == null) { - getParentOperationSet().fireCallEvent( (incomingCall - ? CallEvent.CALL_RECEIVED - : CallEvent.CALL_INITIATED), - this, - mediaDirections); + getParentOperationSet().fireCallEvent( + incomingCall + ? CallEvent.CALL_RECEIVED + : CallEvent.CALL_INITIATED, + this, + mediaDirections); } if(hasZrtp) { callPeer.getMediaHandler().addAdvertisedEncryptionMethod( - SrtpControlType.ZRTP); + SrtpControlType.ZRTP); } - if(hasSdes) { callPeer.getMediaHandler().addAdvertisedEncryptionMethod( - SrtpControlType.SDES); + SrtpControlType.SDES); } } diff --git a/src/net/java/sip/communicator/plugin/sipaccregwizz/SecurityPanel.java b/src/net/java/sip/communicator/plugin/sipaccregwizz/SecurityPanel.java index 2f7c9046c..079788c05 100644 --- a/src/net/java/sip/communicator/plugin/sipaccregwizz/SecurityPanel.java +++ b/src/net/java/sip/communicator/plugin/sipaccregwizz/SecurityPanel.java @@ -29,7 +29,7 @@ public class SecurityPanel */ private static final long serialVersionUID = 0L; - private SIPAccountRegistrationForm regform; + private final SIPAccountRegistrationForm regform; private JPanel pnlAdvancedSettings; private JCheckBox enableDefaultEncryption; @@ -347,30 +347,36 @@ boolean commitPanel(SIPAccountRegistration registration) */ void loadAccount(AccountID accountID) { - enableDefaultEncryption.setSelected(accountID - .getAccountPropertyBoolean( - ProtocolProviderFactory.DEFAULT_ENCRYPTION, true)); - enableSipZrtpAttribute.setSelected(accountID.getAccountPropertyBoolean( - ProtocolProviderFactory.DEFAULT_SIPZRTP_ATTRIBUTE, true)); - cboSavpOption.setSelectedIndex(accountID.getAccountPropertyInt( - ProtocolProviderFactory.SAVP_OPTION, - ProtocolProviderFactory.SAVP_OFF)); - enableSDesAttribute.setSelected(accountID.getAccountPropertyBoolean( - ProtocolProviderFactory.SDES_ENABLED, false)); - cipherModel.loadData(accountID.getAccountPropertyString( - ProtocolProviderFactory.SDES_CIPHER_SUITES)); + enableDefaultEncryption.setSelected( + accountID.getAccountPropertyBoolean( + ProtocolProviderFactory.DEFAULT_ENCRYPTION, + true)); + enableSipZrtpAttribute.setSelected( + accountID.getAccountPropertyBoolean( + ProtocolProviderFactory.DEFAULT_SIPZRTP_ATTRIBUTE, + true)); + cboSavpOption.setSelectedIndex( + accountID.getAccountPropertyInt( + ProtocolProviderFactory.SAVP_OPTION, + ProtocolProviderFactory.SAVP_OFF)); + enableSDesAttribute.setSelected( + accountID.getAccountPropertyBoolean( + ProtocolProviderFactory.SDES_ENABLED, + false)); + cipherModel.loadData( + accountID.getAccountPropertyString( + ProtocolProviderFactory.SDES_CIPHER_SUITES)); loadStates(); } public void actionPerformed(ActionEvent e) { - if(e.getSource() == enableDefaultEncryption - || e.getSource() == enableSDesAttribute) + if((e.getSource() == enableDefaultEncryption) + || (e.getSource() == enableSDesAttribute)) { loadStates(); } - - if(e.getSource() == cmdExpandAdvancedSettings) + else if(e.getSource() == cmdExpandAdvancedSettings) { pnlAdvancedSettings.setVisible(!pnlAdvancedSettings.isVisible()); } diff --git a/src/net/java/sip/communicator/service/neomedia/DefaultTCPStreamConnector.java b/src/net/java/sip/communicator/service/neomedia/DefaultTCPStreamConnector.java index 05aba4239..169ce2c25 100644 --- a/src/net/java/sip/communicator/service/neomedia/DefaultTCPStreamConnector.java +++ b/src/net/java/sip/communicator/service/neomedia/DefaultTCPStreamConnector.java @@ -88,9 +88,9 @@ public void close() if (dataSocket != null) dataSocket.close(); } - catch(IOException e) + catch(IOException ioe) { - logger.debug("Error closing TCP socket", e); + logger.debug("Failed to close TCP socket", ioe); } } diff --git a/src/net/java/sip/communicator/service/neomedia/MediaService.java b/src/net/java/sip/communicator/service/neomedia/MediaService.java index a2812cffd..a2269a965 100644 --- a/src/net/java/sip/communicator/service/neomedia/MediaService.java +++ b/src/net/java/sip/communicator/service/neomedia/MediaService.java @@ -144,16 +144,20 @@ public MediaStream createMediaStream(StreamConnector connector, public MediaFormatFactory getFormatFactory(); /** - * Creates ZrtpControl used to control all zrtp options. + * Initializes a new ZrtpControl instance which is to control all + * ZRTP options. * - * @return ZrtpControl instance. + * @return a new ZrtpControl instance which is to control all ZRTP + * options */ public ZrtpControl createZrtpControl(); /** - * Creates SDesControl used to control all SDes options. + * Initializes a new SDesControl instance which is to control all + * SDes options. * - * @return SDesControl instance. + * @return a new SDesControl instance which is to control all SDes + * options */ public SDesControl createSDesControl(); diff --git a/src/net/java/sip/communicator/service/neomedia/MediaStreamTarget.java b/src/net/java/sip/communicator/service/neomedia/MediaStreamTarget.java index c37068f9d..64488e84f 100644 --- a/src/net/java/sip/communicator/service/neomedia/MediaStreamTarget.java +++ b/src/net/java/sip/communicator/service/neomedia/MediaStreamTarget.java @@ -57,10 +57,7 @@ private boolean addressesAreEqual( InetSocketAddress addr1, InetSocketAddress addr2) { - if (addr1 == null) - return (addr2 == null); - else - return addr1.equals(addr2); + return (addr1 == null) ? (addr2 == null) : addr1.equals(addr2); } /** diff --git a/src/net/java/sip/communicator/service/neomedia/event/SrtpListener.java b/src/net/java/sip/communicator/service/neomedia/event/SrtpListener.java index aa82bb388..7e29f1274 100644 --- a/src/net/java/sip/communicator/service/neomedia/event/SrtpListener.java +++ b/src/net/java/sip/communicator/service/neomedia/event/SrtpListener.java @@ -6,20 +6,20 @@ */ package net.java.sip.communicator.service.neomedia.event; -import net.java.sip.communicator.service.neomedia.SrtpControl; +import net.java.sip.communicator.service.neomedia.*; /** - * The ZrtpListener is meant to be used by the media stream - * creator, as the name indicates in order to be notified when a security event - * has occured that concerns zrt. - * + * The ZrtpListener is meant to be used by the media stream creator, as + * the name indicates in order to be notified when a security event has occurred + * that concerns ZRTP. + * * @author Yana Stamcheva */ public interface SrtpListener { /** * Indicates that the security has been turned on. When we are in the case - * of using multistreams when the master stream zrtp is inited and + * of using multistreams when the master stream ZRTP is initialized and * established the param multiStreamData holds the data needed for the * slave streams to establish their sessions. If this is a securityTurnedOn * event on non master stream the multiStreamData is null. @@ -52,7 +52,7 @@ public void securityMessageReceived(String message, int severity); /** - * Indicates that the other party has timeouted replying to our + * Indicates that the other party has timed out replying to our * offer to secure the connection. * * @param sessionType the type of the call session - audio or video. diff --git a/src/net/java/sip/communicator/service/protocol/Call.java b/src/net/java/sip/communicator/service/protocol/Call.java index a9a401b39..dee43d205 100644 --- a/src/net/java/sip/communicator/service/protocol/Call.java +++ b/src/net/java/sip/communicator/service/protocol/Call.java @@ -77,12 +77,16 @@ protected Call(ProtocolProviderService sourceProvider) this.protocolProvider = sourceProvider; - defaultEncryption = - protocolProvider.getAccountID().getAccountPropertyBoolean( - ProtocolProviderFactory.DEFAULT_ENCRYPTION, true); - sipZrtpAttribute = - protocolProvider.getAccountID().getAccountPropertyBoolean( - ProtocolProviderFactory.DEFAULT_SIPZRTP_ATTRIBUTE, true); + AccountID accountID = protocolProvider.getAccountID(); + + defaultEncryption + = accountID.getAccountPropertyBoolean( + ProtocolProviderFactory.DEFAULT_ENCRYPTION, + true); + sipZrtpAttribute + = accountID.getAccountPropertyBoolean( + ProtocolProviderFactory.DEFAULT_SIPZRTP_ATTRIBUTE, + true); } /** diff --git a/src/net/java/sip/communicator/service/protocol/event/ChatRoomMemberPresenceChangeEvent.java b/src/net/java/sip/communicator/service/protocol/event/ChatRoomMemberPresenceChangeEvent.java index 68a86e0b3..9ca419e48 100644 --- a/src/net/java/sip/communicator/service/protocol/event/ChatRoomMemberPresenceChangeEvent.java +++ b/src/net/java/sip/communicator/service/protocol/event/ChatRoomMemberPresenceChangeEvent.java @@ -15,7 +15,7 @@ * being kicked, join, left... * * @author Emil Ivov - * @author Lubomir Marinov + * @author Lyubomir Marinov */ public class ChatRoomMemberPresenceChangeEvent extends EventObject @@ -61,13 +61,6 @@ public class ChatRoomMemberPresenceChangeEvent */ private final ChatRoomMember sourceMember; - /** - * The chat room member that participated as an actor in this event. In the - * case of MEMBER_KICKED event this would be the moderator that kicked the - * participant. - */ - private final ChatRoomMember actorMember; - /** * The type of this event. Values can be any of the MEMBER_XXX fields. */ @@ -103,9 +96,11 @@ public ChatRoomMemberPresenceChangeEvent( ChatRoom sourceRoom, * Changes may include the participant being kicked, join, left, etc. * * @param sourceRoom the ChatRoom that produced this event - * @param sourceMember the ChatRoomMember that this event is about - * @param actorMember the ChatRoomMember that participated as an - * actor in this event + * @param sourceMember the ChatRoomMember who this event is about + * @param actorMember the ChatRoomMember who participated as an + * actor in the new event. For example, in the case of a + * MEMBER_KICKED event the actorMember is the moderator + * who kicked the sourceMember. * @param eventType the event type; one of the MEMBER_XXX constants * @param reason the reason explaining why this event might have occurred */ @@ -117,7 +112,7 @@ public ChatRoomMemberPresenceChangeEvent( ChatRoom sourceRoom, { super(sourceRoom); this.sourceMember = sourceMember; - this.actorMember = actorMember; + // this.actorMember = actorMember; this.eventType = eventType; this.reason = reason; } diff --git a/src/net/java/sip/communicator/service/protocol/media/AbstractQualityControlWrapper.java b/src/net/java/sip/communicator/service/protocol/media/AbstractQualityControlWrapper.java index fc2d15ce0..c8114e9de 100644 --- a/src/net/java/sip/communicator/service/protocol/media/AbstractQualityControlWrapper.java +++ b/src/net/java/sip/communicator/service/protocol/media/AbstractQualityControlWrapper.java @@ -23,7 +23,7 @@ public abstract class AbstractQualityControlWrapper< /** * The peer we are controlling. */ - protected T peer; + protected final T peer; /** * The media quality control. @@ -60,7 +60,7 @@ protected QualityControl getMediaQualityControl() MediaStream stream = peer.getMediaHandler().getStream(MediaType.VIDEO); - if(stream != null && stream instanceof VideoMediaStream) + if(stream instanceof VideoMediaStream) qualityControl = ((VideoMediaStream)stream).getQualityControl(); return qualityControl; diff --git a/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java b/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java index 283872040..be9c8c693 100644 --- a/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java +++ b/src/net/java/sip/communicator/service/protocol/media/MediaAwareCallPeer.java @@ -660,15 +660,13 @@ public void audioLevelsReceived(long[] audioLevels) * Does nothing. * @param evt the event. */ - public void callPeerAdded(CallPeerEvent evt) - {} + public void callPeerAdded(CallPeerEvent evt) {} /** * Does nothing. * @param evt the event. */ - public void callPeerRemoved(CallPeerEvent evt) - {} + public void callPeerRemoved(CallPeerEvent evt) {} /** * Sets the security status to ON for this call peer. @@ -681,9 +679,8 @@ public void securityTurnedOn(int sessionType, String cipher, SrtpControl sender) { getMediaHandler().startSrtpMultistream(sender); - CallPeerSecurityOnEvent evt = - new CallPeerSecurityOnEvent(this, sessionType, cipher, sender); - fireCallPeerSecurityOnEvent(evt); + fireCallPeerSecurityOnEvent( + new CallPeerSecurityOnEvent(this, sessionType, cipher, sender)); } /** diff --git a/src/net/java/sip/communicator/service/protocol/media/TransportManager.java b/src/net/java/sip/communicator/service/protocol/media/TransportManager.java index ea0bd3002..5758e8480 100644 --- a/src/net/java/sip/communicator/service/protocol/media/TransportManager.java +++ b/src/net/java/sip/communicator/service/protocol/media/TransportManager.java @@ -6,7 +6,6 @@ */ package net.java.sip.communicator.service.protocol.media; -import java.io.*; import java.net.*; import java.util.*; @@ -163,40 +162,19 @@ else if(streamConnector.getProtocol() == StreamConnector.Protocol.TCP) */ public void closeStreamConnector(MediaType mediaType) { - StreamConnector connector - = streamConnectors[mediaType.ordinal()]; + int index = mediaType.ordinal(); + StreamConnector connector = streamConnectors[index]; - if(connector != null) + if (connector != null) { - synchronized(connector) - { - DatagramSocket dataSocket = connector.getDataSocket(); - - if (dataSocket != null) - dataSocket.close(); - - DatagramSocket controlSocket = connector.getControlSocket(); - - if (controlSocket != null) - controlSocket.close(); - - try - { - Socket dataTcpSocket = connector.getDataTCPSocket(); - if(dataTcpSocket != null) - dataTcpSocket.close(); - - Socket controlTcpSocket = connector.getDataTCPSocket(); - if(controlTcpSocket != null) - controlTcpSocket.close(); - } - catch(IOException e) - { - logger.info("Failed to close TCP socket", e); - } - - streamConnectors[mediaType.ordinal()] = null; - } + /* + * XXX The connected owns the sockets so it is important that it + * decides whether to close them i.e. this TransportManager is not + * allowed to explicitly close the sockets by itself. + */ + connector.close(); + + streamConnectors[index] = null; } } @@ -208,7 +186,7 @@ public void closeStreamConnector(MediaType mediaType) * StreamConnector is to be created * @return a new StreamConnector. * - * @throws OperationFailedException if we fail binding the the sockets. + * @throws OperationFailedException if the binding of the sockets fails. */ protected StreamConnector createStreamConnector(MediaType mediaType) throws OperationFailedException @@ -441,9 +419,10 @@ protected void setTrafficClass(MediaStreamTarget target, MediaType type) { connector.getDataTCPSocket().setTrafficClass(trafficClass); - if(connector.getControlTCPSocket() != null) - connector.getControlTCPSocket(). - setTrafficClass(trafficClass); + Socket controlTCPSocket = connector.getControlTCPSocket(); + + if (controlTCPSocket != null) + controlTCPSocket.setTrafficClass(trafficClass); } else { @@ -451,9 +430,10 @@ protected void setTrafficClass(MediaStreamTarget target, MediaType type) connector.getDataSocket().setTrafficClass(trafficClass); /* control port (RTCP) */ - if(connector.getControlSocket() != null) - connector.getControlSocket().setTrafficClass( - trafficClass); + DatagramSocket controlSocket = connector.getControlSocket(); + + if (controlSocket != null) + controlSocket.setTrafficClass(trafficClass); } } } diff --git a/src/net/java/sip/communicator/util/swing/SIPCommDialog.java b/src/net/java/sip/communicator/util/swing/SIPCommDialog.java index 27102f81f..282d4a4a4 100644 --- a/src/net/java/sip/communicator/util/swing/SIPCommDialog.java +++ b/src/net/java/sip/communicator/util/swing/SIPCommDialog.java @@ -434,11 +434,11 @@ public void dispose() /** * All functions implemented in this method will be invoked when user * presses the Escape key. - * @param isEscaped indicates if this frame has been closed by pressing the - * Esc key; otherwise, false + * + * @param escaped true if this frame has been closed by pressing + * the Esc key; otherwise, false */ - protected void close(boolean isEscaped) + protected void close(boolean escaped) { - } }