[SNMP4J] Counting Invalid Traps

[Frank Fock]

Hi Elise,

Thank you for reporting this bug [SJF-18].
The current snapshot contains a fix for it,
which is slightly different to yours.

Regards,
Frank

Elise Atkins wrote:
> I have used SNMP4J for several years and have been very satisfied. 
> Recently I did some testing to see what happens when invalid V1 traps 
> and snmp requests were received by the snmp4j code. A CounterListner was 
> added to the MessageDispatcherImpl and the number of incoming 
> traps/requests were accurately counted but the number of invalid traps 
> was not. The large majority of traps were silently discarded. After 
> looking at the following code snippet from MessageDispatcherImpl and the 
> MPv1.java code I concluded that when the MPv1 encountered an error it 
> threw an exception that was caught by the catch block near the end of 
> the snippet. The packet is then silently discarded unless the 
> Snmp4JSettings.forwardRuntimeExceptions is true and the 
> TransportMapping  class catches and counts the exception.
> 
> I think that a better solution would be to add the following lines the 
> catch block:
>         CounterEvent event = new CounterEvent(this,
>                                               
> SnmpConstants.snmpInvalidMsgs);
>         fireIncrementCounter(event);
> 
> The packet would be then counted as invalid. I did set the 
> forwardRuntimeExceptions to true and was able to get a count of the 
> invalid packets and all seemed ok until a little later. I also use 
> AgentXSubagent to connect to the snmpd service. When that service was 
> not running or misconfigured and the subagent could not connect, a 
> socket was orphaned and eventually things stopped working because there 
> were too many sockets opened. I fixed that by calling close and 
> disconnect on the subagent when the connect method failed. But I am now 
> worried that having the forwardRuntimeExceptions set to true will cause 
> additional problems down the line.
> 
>   public void processMessage(TransportMapping sourceTransport,
>                              Address incomingAddress,
>                              BERInputStream wholeMessage) {
>     fireIncrementCounter(new CounterEvent(this, SnmpConstants.snmpInPkts));
>     if (!wholeMessage.markSupported()) {
>       String txt = "Message stream must support marks";
>       logger.error(txt);
>       throw new IllegalArgumentException(txt);
>     }
>     try {
>       wholeMessage.mark(16);
>       BER.MutableByte type = new BER.MutableByte();
>       // decode header but do not check length here, because we do only 
> decode
>       // the first 16 bytes.
>       BER.decodeHeader(wholeMessage, type, false);
>       if (type.getValue() != BER.SEQUENCE) {
>         logger.error("ASN.1 parse error (message is not a sequence)");
>         CounterEvent event = new CounterEvent(this,
>                                               
> SnmpConstants.snmpInASNParseErrs);
>         fireIncrementCounter(event);
>       }
>       Integer32 version = new Integer32();
>       version.decodeBER(wholeMessage);
>       MessageProcessingModel mp = 
> getMessageProcessingModel(version.getValue());
>       if (mp == null) {
>         logger.warn("SNMP version "+version+" is not supported");
>         CounterEvent event = new CounterEvent(this,
>                                               
> SnmpConstants.snmpInBadVersions);
>         fireIncrementCounter(event);
>       }
>       else {
>         // reset it
>         wholeMessage.reset();
>         // dispatch it
>         dispatchMessage(sourceTransport, mp, incomingAddress, wholeMessage);
>       }
>     }
>     catch (Exception ex) {
>       logger.error(ex);
>       if (logger.isDebugEnabled()) {
>         ex.printStackTrace();
>       }
>       if (SNMP4JSettings.isFowardRuntimeExceptions()) {
>         throw new RuntimeException(ex);
>       }
>     }
>     catch (OutOfMemoryError oex) {
>       logger.error(oex);
>       if (SNMP4JSettings.isFowardRuntimeExceptions()) {
>         throw oex;
>       }
>     }
>   }
> 
> Sincerely,
> Elise Atkins
> 

-- 
AGENT++
http://www.agentpp.com
http://www.snmp4j.com
http://www.mibexplorer.com
http://www.mibdesigner.com